Repository: obsidian-tasks-group/obsidian-tasks
Pull request #909 (2022-07-19 09:18:00)

fix: Show any markdown in group headings correctly
<!--- Provide a general summary of your changes in the Title above -->

## Description
<!--- Describe your changes in detail -->

The code that displayed group headings was treating them as plain text, meaning that any markdown inside them was also drawn as plain text.

This instead passes the heading text through `MarkdownRenderer.renderMarkdown()`.

### Note for reviewers

~~I'm a bit unsure about the last argument `null as unknown as Component` passed to `MarkdownRenderer.renderMarkdown()`.~~

~~There is probably some available object in `QueryRenderer.ts` that I should be passing one.~~

Fixed.

## Motivation and Context

This fixes the question raised in discussion #907.


<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->

## How has this been tested?

By running a local build of the change, using the tasks and query in comment https://github.com/obsidian-tasks-group/obsidian-tasks/discussions/907#discussioncomment-3177639

<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, tests ran to see how -->
<!--- your change affects other areas of the code, etc. -->

## Screenshots (if appropriate):


![](https://user-images.githubusercontent.com/4840096/179685991-85ec2109-7dd9-4e45-873f-a44fa1807610.png)


## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] Refactor (non-breaking change which only improves the design or structure of existing code, and making no changes to its external behaviour)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)

## 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`.
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [ ] My change has adequate Unit Test coverage.

By creating a Pull Request you agree to our [Code of Conduct](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CODE_OF_CONDUCT.md). For further guidance on contributing please see [contributing guide](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CONTRIBUTING.md)


claremacrae: I am slightly inclined to merge the PR as is, on the grounds that it is an improvement, as it allows clicking on links inside group headings, and is the simplest thing to do.

And wait to see if anyone complains about its impact on the rendering of any other headings, like on file names.

claremacrae: Regarding the back links, I'm reasonably comfortable with not changing their display, as they have existed for much longer than grouping, and to my knowledge, nobody has complained about them not being rendered.
claremacrae: > It is an improvement for `heading`, but a regression for `filename`, `path`, and (I have not kept up with all the new grouping options) maybe `root` too. 

Ah right, OK thanks. In that case I will hold off and try to think of another way. One idea might be to try and escape or protect characters that could be markup in all fields except heading. So that when their names are sent though `renderMarkdown()` for headings they will still display as they currently do.
claremacrae: > @claremacrae or someone else who has "Insider" - it looks like Obsidian 0.16 changes approved punctuation in headings, which hopefully will fix some of the backlink issues?
> 
> So the right approach here might be to wait until Obsidian 0.16 changes stabilize, and then tackle this again. Unfortunately I will not be available to review the PR then.

Thanks Anna. I think I would separate those out in to two issues - see how Tasks' backlinks work with 0.16 - and then make sure that group headings are rendered correctly.

Unfortunately I've reverted to pre 0.16 due to its breaking changes in keyboard modifiers breaking my normal workflow.
claremacrae: > @claremacrae: Sounds good to me! Is there anything else you need from me on this issue/PR before I disappear back into school work?

No, thank you!


claremacrae: @AnnaKornfeldSimpson I think I have improved this to the point where it is good enough to release...

Changes made:

- Any headings that contain paths or parts of paths now have `\` and `_` escaped
- This prevents the issue that underscores in file names no longer create italics
- Lots more tests added
- Sample file added to Tasks-Demo vault
- Added comments saying that the group names are now all rendered, so non-markdown characters are supposed to be escaped 

If there are complaints in future with real-world examples of other characters needing escaping, I may adjust the code to use https://www.npmjs.com/package/markdown-escape, but I did not want to add another dependency at this point.

## Old behaviour with sample file

![image](https://user-images.githubusercontent.com/4840096/188509985-f463104f-c1ac-4342-be27-264ece5b1315.png)


## New behaviour with sample file

![image](https://user-images.githubusercontent.com/4840096/188509947-ea19c978-7325-4d40-8ffa-95acb7f40735.png)


Patch:
diff --git a/src/Query/Group.ts b/src/Query/Group.ts
index 3660f9ee..ea1f1b34 100644
--- a/src/Query/Group.ts
+++ b/src/Query/Group.ts
@@ -26,6 +26,9 @@ export class Group {
 
     /**
      * Return the properties of a single task for the passed grouping property
+     *
+     * The returned string will be rendered, so any special Markdown characters will be escaped
+     *
      * @param property
      * @param task
      */
@@ -56,6 +59,11 @@ export class Group {
         tags: Group.groupByTags,
     };
 
+    private static escapeMarkdownCharacters(filename: string) {
+        // https://wilsonmar.github.io/markdown-text-for-github-from-html/#special-characters
+        return filename.replace(/\\/g, '\\\\').replace(/_/g, '\\_');
+    }
+
     private static groupByPriority(task: Task): string[] {
         let priorityName = 'ERROR';
         switch (task.priority) {
@@ -125,7 +133,7 @@ export class Group {
     private static groupByPath(task: Task): string[] {
         // Does this need to be made stricter?
         // Is there a better way of getting the file name?
-        return [task.path.replace('.md', '')];
+        return [Group.escapeMarkdownCharacters(task.path.replace('.md', ''))];
     }
 
     private static groupByFolder(task: Task): string[] {
@@ -138,7 +146,7 @@ export class Group {
         if (folder === '') {
             return ['/'];
         }
-        return [folder];
+        return [Group.escapeMarkdownCharacters(folder)];
     }
 
     private static groupByFileName(task: Task): string[] {
@@ -149,7 +157,7 @@ export class Group {
         if (filename === null) {
             return ['Unknown Location'];
         }
-        return [filename];
+        return [Group.escapeMarkdownCharacters(filename)];
     }
 
     private static groupByRoot(task: Task): string[] {
@@ -158,7 +166,11 @@ export class Group {
         if (separatorIndex == -1) {
             return ['/'];
         }
-        return [path.substring(0, separatorIndex + 1)];
+        return [
+            Group.escapeMarkdownCharacters(
+                path.substring(0, separatorIndex + 1),
+            ),
+        ];
     }
 
     private static groupByBacklink(task: Task): string[] {
@@ -166,7 +178,23 @@ export class Group {
         if (linkText === null) {
             return ['Unknown Location'];
         }
-        return [linkText];
+
+        // Markdown characters in the file name must be escaped.
+        // Markdown characters in the heading must NOT be escaped.
+        const filenameComponent = Group.groupByFileName(task)[0];
+        if (
+            task.precedingHeader === null ||
+            task.precedingHeader.length === 0
+        ) {
+            return [filenameComponent];
+        }
+        const headingComponent = Group.groupByHeading(task)[0];
+
+        if (filenameComponent === headingComponent) {
+            return [filenameComponent];
+        } else {
+            return [`${filenameComponent} > ${headingComponent}`];
+        }
     }
 
     private static groupByStatus(task: Task): string[] {
diff --git a/src/QueryRenderer.ts b/src/QueryRenderer.ts
index 3d769003..1da1614f 100644
--- a/src/QueryRenderer.ts
+++ b/src/QueryRenderer.ts
@@ -1,4 +1,10 @@
-import { App, MarkdownRenderChild, Plugin, TFile } from 'obsidian';
+import {
+    App,
+    MarkdownRenderChild,
+    MarkdownRenderer,
+    Plugin,
+    TFile,
+} from 'obsidian';
 import type { EventRef, MarkdownPostProcessorContext } from 'obsidian';
 
 import type { IQuery } from './IQuery';
@@ -148,7 +154,7 @@ class QueryRenderChild extends MarkdownRenderChild {
             for (const group of tasksSortedLimitedGrouped.groups) {
                 // If there were no 'group by' instructions, group.groupHeadings
                 // will be empty, and no headings will be added.
-                QueryRenderChild.addGroupHeadings(content, group.groupHeadings);
+                this.addGroupHeadings(content, group.groupHeadings);
 
                 const { taskList } = await this.createTasksList({
                     tasks: group.tasks,
@@ -247,16 +253,16 @@ class QueryRenderChild extends MarkdownRenderChild {
      *                        in which case no headings will be added.
      * @private
      */
-    private static addGroupHeadings(
+    private addGroupHeadings(
         content: HTMLDivElement,
         groupHeadings: GroupHeading[],
     ) {
         for (const heading of groupHeadings) {
-            QueryRenderChild.addGroupHeading(content, heading);
+            this.addGroupHeading(content, heading);
         }
     }
 
-    private static addGroupHeading(
+    private async addGroupHeading(
         content: HTMLDivElement,
         group: GroupHeading,
     ) {
@@ -278,7 +284,12 @@ class QueryRenderChild extends MarkdownRenderChild {
                 cls: 'tasks-group-heading',
             });
         }
-        header.appendText(group.name);
+        await MarkdownRenderer.renderMarkdown(
+            group.name,
+            header,
+            this.filePath,
+            this,
+        );
     }
 
     private addBacklinks(


Test patch:
diff --git a/resources/sample_vaults/Tasks-Demo/Manual Testing/Markdown in group _non-italic filename_.md b/resources/sample_vaults/Tasks-Demo/Manual Testing/Markdown in group _non-italic filename_.md
new file mode 100644
index 00000000..df8a19f7
--- /dev/null
+++ b/resources/sample_vaults/Tasks-Demo/Manual Testing/Markdown in group _non-italic filename_.md
@@ -0,0 +1,40 @@
+# Markdown in group headings - _italic text in heading_
+
+Thanks to check:
+
+- The text in  `_` in the file name should not be displayed as italic, in group headings
+- The text in `_` in the H1 in this file should be rendered as italic
+
+## 1 Heading with percent comment %% percent %%
+
+- [ ] #task Task in Heading with percent comment
+
+## 2 Heading with HTML comment <!-- HTML Comment -->
+
+- [ ] #task Task in Heading with HTML comment
+
+## 3 Heading with link to internal page [[README - Manual Testing]]
+
+- [ ] #task Heading with link to internal page
+
+## 4 Heading with link to web page [#907](https://github.com/obsidian-tasks-group/obsidian-tasks/discussions/907)
+
+- [ ] #task Heading with link to web page
+
+## 5 Some _italic text in heading_
+
+- [ ] #task Heading has italic text
+
+---
+
+## 6 Tasks in This File
+
+> [!todo]
+>
+> ```tasks
+> path includes Markdown in group _non-italic filename_
+> 
+> group by backlink
+> group by filename
+> group by heading
+> ```
diff --git a/tests/Group.test.ts b/tests/Group.test.ts
index 7963edc7..194762bd 100644
--- a/tests/Group.test.ts
+++ b/tests/Group.test.ts
@@ -200,22 +200,22 @@ describe('Grouping tasks', () => {
         // Assert
         expect(groups.toString()).toMatchInlineSnapshot(`
             "
-            Group names: [folder_a/folder_b/,file_c]
-            #### folder_a/folder_b/
-            ##### file_c
+            Group names: [folder\\\\_a/folder\\\\_b/,file\\\\_c]
+            #### folder\\\\_a/folder\\\\_b/
+            ##### file\\\\_c
             - [ ] Task 3 - but path is 1st, alphabetically
 
             ---
 
-            Group names: [folder_b/folder_c/,file_c]
-            #### folder_b/folder_c/
-            ##### file_c
+            Group names: [folder\\\\_b/folder\\\\_c/,file\\\\_c]
+            #### folder\\\\_b/folder\\\\_c/
+            ##### file\\\\_c
             - [ ] Task 1 - but path is 2nd, alphabetically
 
             ---
 
-            Group names: [folder_b/folder_c/,file_d]
-            ##### file_d
+            Group names: [folder\\\\_b/folder\\\\_c/,file\\\\_d]
+            ##### file\\\\_d
             - [ ] Task 2 - but path is 2nd, alphabetically
 
             ---
@@ -240,6 +240,18 @@ describe('Group names', () => {
 
         // -----------------------------------------------------------
         // group by backlink
+        {
+            groupBy: 'backlink',
+            taskLine: '- [ ] xxx', // no location supplied
+            expectedGroupNames: ['Unknown Location'],
+            precedingHeading: 'heading',
+        },
+        {
+            groupBy: 'backlink',
+            taskLine: '- [ ] xxx', // no heading supplied
+            expectedGroupNames: ['c'],
+            path: 'a/b/c.md',
+        },
         {
             groupBy: 'backlink',
             taskLine: '- [ ] xxx',
@@ -247,6 +259,22 @@ describe('Group names', () => {
             path: 'a/b/c.md',
             precedingHeading: 'heading',
         },
+        {
+            groupBy: 'backlink',
+            taskLine: '- [ ] xxx',
+            expectedGroupNames: ['c'], // If file name and heading are identical, avoid duplication ('c > c')
+            path: 'a/b/c.md',
+            precedingHeading: 'c',
+        },
+        {
+            groupBy: 'backlink',
+            taskLine: '- [ ] xxx',
+            // underscores in file name component are escaped
+            // but underscores in the heading component are not
+            expectedGroupNames: ['\\_c\\_ > heading _italic text_'],
+            path: 'a/b/_c_.md',
+            precedingHeading: 'heading _italic text_',
+        },
 
         // -----------------------------------------------------------
         // group by done
@@ -282,6 +310,12 @@ describe('Group names', () => {
             expectedGroupNames: ['c'],
             path: 'a/b/c.md',
         },
+        {
+            groupBy: 'filename',
+            taskLine: '- [ ] a',
+            expectedGroupNames: ['\\_c\\_'], // underscores in file names are escaped
+            path: 'a/b/_c_.md',
+        },
 
         // -----------------------------------------------------------
         // group by folder
@@ -291,6 +325,12 @@ describe('Group names', () => {
             expectedGroupNames: ['a/b/'],
             path: 'a/b/c.md',
         },
+        {
+            groupBy: 'folder',
+            taskLine: '- [ ] a',
+            expectedGroupNames: ['a/\\_b\\_/'], // underscores in folder names are escaped
+            path: 'a/_b_/c.md',
+        },
         {
             // file in root of vault:
             groupBy: 'folder',
@@ -363,6 +403,12 @@ describe('Group names', () => {
             expectedGroupNames: ['heading'],
             precedingHeading: 'heading',
         },
+        {
+            groupBy: 'heading',
+            taskLine: '- [ ] xxx',
+            expectedGroupNames: ['heading _italic text_'], // underscores in headings are NOT escaped - will be rendered
+            precedingHeading: 'heading _italic text_',
+        },
 
         // -----------------------------------------------------------
         // group by path
@@ -370,7 +416,19 @@ describe('Group names', () => {
             groupBy: 'path',
             taskLine: '- [ ] a',
             path: 'a/b/c.md',
-            expectedGroupNames: ['a/b/c'],
+            expectedGroupNames: ['a/b/c'], // the file extension is removed
+        },
+        {
+            groupBy: 'path',
+            taskLine: '- [ ] a',
+            path: '_a_/b/_c_.md',
+            expectedGroupNames: ['\\_a\\_/b/\\_c\\_'], // underscores in paths are escaped
+        },
+        {
+            groupBy: 'path',
+            taskLine: '- [ ] a',
+            path: 'a\\b\\c.md',
+            expectedGroupNames: ['a\\\\b\\\\c'], // backslashes are escaped. (this artificial example is to test escaping)
         },
 
         // -----------------------------------------------------------
@@ -440,11 +498,17 @@ describe('Group names', () => {
             expectedGroupNames: ['a/'],
             path: 'a/b/c.md',
         },
+        {
+            groupBy: 'root',
+            taskLine: '- [ ] a',
+            expectedGroupNames: ['\\_g\\_/'], // underscores in root folder names are escaped
+            path: '_g_/h/i.md',
+        },
         {
             groupBy: 'root',
             taskLine: '- [ ] a',
             expectedGroupNames: ['a/'],
-            path: 'a\\b\\c.md',
+            path: 'a\\b\\c.md', // Windows path
         },
         {
             // file in root of vault: