feat: "Review and check your Statuses" now shows any problems (#2399)

* feat: Add (empty for now) column 'Problems (if any)' * feat: Report any empty status symbols in settings * feat: Report any duplicate status symbols in settings * feat: Report any unknown next status symbols in settings * feat: Report DONE statuses whose next status type is a problem for recurrence See #2089 and #2304 * feat: Add a gentle message for statuses with non-conventional type. * refactor: . Unwrap else block, to reduce indentation. * refactor: - Flip if and then unwrap else block, to reduce indentation. * fix: Make punctuation consistent at end of messages. * fix: Format values in messages, so that they stand out. * fix: Correct the display of empty symbols. They were rendered in Obsidian as `` * refactor: . Flip if condition to reduce nesting. * refactor: . Use early return to reduce nesting. * docs: I've realised that recurring DONE to IN_PROGRESS is OK So update the heading in the relevant docs page. See #2304 (comment) * feat: Add link to page 'Recurring Tasks and Custom Statuses' * feat:
obsidian-tasks-group · Nov 6, 2023 · e8edb72 · e8edb72
1 parent aa4a745
commit e8edb72
Show file tree

Hide file tree

Showing 7 changed files with 148 additions and 19 deletions.
diff --git a/docs/Getting Started/Statuses/Recurring Tasks and Custom Statuses.md b/docs/Getting Started/Statuses/Recurring Tasks and Custom Statuses.md
@@ -100,7 +100,7 @@ Let's look at the **second of those two lines**:
   - ... because the next status symbol after `x` is ``.
 - And the Due date has advanced a day.
 
-## When DONE is not followed by TODO
+## When DONE is not followed by TODO or IN_PROGRESS
 
 In the following example, `DONE` is followed by `CANCELLED`.
 

diff --git a/src/StatusRegistryReport.ts b/src/StatusRegistryReport.ts
@@ -49,6 +49,12 @@ ${mermaidText}`;
 }
 
 export function getPrintableSymbol(symbol: string) {
+    // Do not put backticks around an empty symbol, as the two backticks are rendered
+    // by Obsidian as ordinary characters and the meaning is unclear.
+    // Better to just display nothing in this situation.
+    if (symbol === '') {
+        return symbol;
+    }
     const result = symbol !== ' ' ? symbol : 'space';
     return '`' + result + '`';
 }
diff --git a/src/StatusSettingsReport.ts b/src/StatusSettingsReport.ts
@@ -2,21 +2,105 @@ import { StatusSettings } from './Config/StatusSettings';
 import { MarkdownTable } from './lib/MarkdownTable';
 import type { StatusConfiguration } from './StatusConfiguration';
 import { getPrintableSymbol } from './StatusRegistryReport';
+import { Status } from './Status';
+import { StatusType } from './StatusConfiguration';
+
+function getFirstIndex(statusConfigurations: StatusConfiguration[], wantedSymbol: string) {
+    return statusConfigurations.findIndex((s) => s.symbol === wantedSymbol);
+}
+
+function checkIfConventionalType(status: StatusConfiguration, problems: string[]) {
+    // Check if conventional type is being used:
+    const conventionalType = Status.getTypeForUnknownSymbol(status.symbol);
+    if (status.type === conventionalType) {
+        return;
+    }
+
+    if (conventionalType === StatusType.TODO && status.symbol !== ' ') {
+        // This was likely a default TODO - ignore it.
+        return;
+    }
+
+    problems.push(
+        `For information, the conventional type for status symbol ${getPrintableSymbol(
+            status.symbol,
+        )} is ${getPrintableSymbol(conventionalType)}: you may wish to review this type.`,
+    );
+}
+
+function checkNextStatusSymbol(statuses: StatusConfiguration[], status: StatusConfiguration, problems: string[]) {
+    // Check if next symbol is known
+    const indexOfNextSymbol = getFirstIndex(statuses, status.nextStatusSymbol);
+    if (indexOfNextSymbol === -1) {
+        problems.push(
+            `Next symbol ${getPrintableSymbol(
+                status.nextStatusSymbol,
+            )} is unknown: create a status with symbol ${getPrintableSymbol(status.nextStatusSymbol)}.`,
+        );
+        return;
+    }
+
+    if (status.type !== StatusType.DONE) {
+        return;
+    }
+
+    // This type is DONE: check that next status type is TODO or IN_PROGRESS.
+    // See issues #2089 and #2304.
+    const nextStatus = statuses[indexOfNextSymbol];
+    if (nextStatus) {
+        if (nextStatus.type !== 'TODO' && nextStatus.type !== 'IN_PROGRESS') {
+            const helpURL =
+                'https://publish.obsidian.md/tasks/Getting+Started/Statuses/Recurring+Tasks+and+Custom+Statuses';
+            problems.push(
+                `This \`DONE\` status is followed by ${getPrintableSymbol(
+                    nextStatus.type,
+                )}, not \`TODO\` or \`IN_PROGRESS\`: this will not work well for recurring tasks. See [Recurring Tasks and Custom Statuses](${helpURL}).`,
+            );
+        }
+    } else {
+        problems.push('Unexpected failure to find the next status.');
+    }
+}
+
+function getProblemsForStatus(statuses: StatusConfiguration[], status: StatusConfiguration, index: number) {
+    const problems: string[] = [];
+    if (status.symbol === Status.EMPTY.symbol) {
+        problems.push('Empty symbol: this status will be ignored.');
+        return problems;
+    }
+
+    const firstIndex = getFirstIndex(statuses, status.symbol);
+    if (firstIndex != index) {
+        problems.push(`Duplicate symbol '${getPrintableSymbol(status.symbol)}': this status will be ignored.`);
+        return problems;
+    }
+
+    checkIfConventionalType(status, problems);
+    checkNextStatusSymbol(statuses, status, problems);
+    return problems;
+}
 
 export function tabulateStatusSettings(statusSettings: StatusSettings) {
     // Note: There is very similar code in verifyStatusesAsMarkdownTable() in DocsSamplesForStatuses.test.ts.
     //       Maybe try unifying the common code one day?
 
-    const table = new MarkdownTable(['Status Symbol', 'Next Status Symbol', 'Status Name', 'Status Type']);
+    const table = new MarkdownTable([
+        'Status Symbol',
+        'Next Status Symbol',
+        'Status Name',
+        'Status Type',
+        'Problems (if any)',
+    ]);
 
     const statuses: StatusConfiguration[] = StatusSettings.allStatuses(statusSettings);
-    for (const status of statuses) {
+    statuses.forEach((status, index) => {
         table.addRow([
             getPrintableSymbol(status.symbol),
             getPrintableSymbol(status.nextStatusSymbol),
             status.name,
             getPrintableSymbol(status.type),
+            getProblemsForStatus(statuses, status, index).join('<br>'),
         ]);
-    }
+    });
     return table.markdown;
 }
diff --git a/...atusRegistryReport.test.StatusRegistryReport_should_create_a_report.approved.md b/...atusRegistryReport.test.StatusRegistryReport_should_create_a_report.approved.md
@@ -21,15 +21,15 @@ Obsidian may only render the table correctly in Reading Mode.
 
 These are the status values in the Core and Custom statuses sections.
 
-| Status Symbol | Next Status Symbol | Status Name | Status Type |
-| ----- | ----- | ----- | ----- |
-| `space` | `x` | Todo | `TODO` |
-| `x` | `space` | Done | `DONE` |
-| `/` | `x` | In Progress | `IN_PROGRESS` |
-| `-` | `space` | Cancelled | `CANCELLED` |
-| `Q` | `A` | Question | `NON_TASK` |
-| `A` | `Q` | Answer | `NON_TASK` |
-| `` | `` |  | `TODO` |
+| Status Symbol | Next Status Symbol | Status Name | Status Type | Problems (if any) |
+| ----- | ----- | ----- | ----- | ----- |
+| `space` | `x` | Todo | `TODO` |  |
+| `x` | `space` | Done | `DONE` |  |
+| `/` | `x` | In Progress | `IN_PROGRESS` |  |
+| `-` | `space` | Cancelled | `CANCELLED` |  |
+| `Q` | `A` | Question | `NON_TASK` |  |
+| `A` | `Q` | Answer | `NON_TASK` |  |
+|  |  |  | `TODO` | Empty symbol: this status will be ignored. |
 
 ## Loaded Settings
 

diff --git a/...gsReport.test.StatusSettingsReport_should_include_problems_in_table.approved.md b/...gsReport.test.StatusSettingsReport_should_include_problems_in_table.approved.md
@@ -0,0 +1,16 @@
+| Status Symbol | Next Status Symbol | Status Name | Status Type | Problems (if any) |
+| ----- | ----- | ----- | ----- | ----- |
+| `space` | `x` | Todo | `TODO` |  |
+| `x` | `space` | Done | `DONE` |  |
+| `/` | `x` | In Progress | `IN_PROGRESS` |  |
+| `/` | `x` | In Progress DUPLICATE | `IN_PROGRESS` | Duplicate symbol '`/`': this status will be ignored. |
+| `X` | `space` | X - conventionally DONE, but this is CANCELLED | `CANCELLED` | For information, the conventional type for status symbol `X` is `DONE`: you may wish to review this type. |
+|  |  |  | `TODO` | Empty symbol: this status will be ignored. |
+| `p` | `q` | Unknown next symbol | `TODO` | Next symbol `q` is unknown: create a status with symbol `q`. |
+| `c` | `d` | Followed by d | `TODO` | Next symbol `d` is unknown: create a status with symbol `d`. |
+| `n` | `n` | Non-task | `NON_TASK` |  |
+| `1` | `space` | DONE followed by TODO | `DONE` |  |
+| `2` | `/` | DONE followed by IN_PROGRESS | `DONE` |  |
+| `3` | `x` | DONE followed by DONE | `DONE` | This `DONE` status is followed by `DONE`, not `TODO` or `IN_PROGRESS`: this will not work well for recurring tasks. See [Recurring Tasks and Custom Statuses](https://publish.obsidian.md/tasks/Getting+Started/Statuses/Recurring+Tasks+and+Custom+Statuses). |
+| `4` | `X` | DONE followed by CANCELLED | `DONE` | This `DONE` status is followed by `CANCELLED`, not `TODO` or `IN_PROGRESS`: this will not work well for recurring tasks. See [Recurring Tasks and Custom Statuses](https://publish.obsidian.md/tasks/Getting+Started/Statuses/Recurring+Tasks+and+Custom+Statuses). |
+| `5` | `n` | DONE followed by NON_TASK | `DONE` | This `DONE` status is followed by `NON_TASK`, not `TODO` or `IN_PROGRESS`: this will not work well for recurring tasks. See [Recurring Tasks and Custom Statuses](https://publish.obsidian.md/tasks/Getting+Started/Statuses/Recurring+Tasks+and+Custom+Statuses). |
diff --git a/...ingsReport.test.StatusSettingsReport_should_tabulate_StatusSettings.approved.md b/...ingsReport.test.StatusSettingsReport_should_tabulate_StatusSettings.approved.md
@@ -1,6 +1,6 @@
-| Status Symbol | Next Status Symbol | Status Name | Status Type |
-| ----- | ----- | ----- | ----- |
-| `space` | `x` | Todo | `TODO` |
-| `x` | `space` | Done | `DONE` |
-| `/` | `x` | In Progress | `IN_PROGRESS` |
-| `-` | `space` | Cancelled | `CANCELLED` |
+| Status Symbol | Next Status Symbol | Status Name | Status Type | Problems (if any) |
+| ----- | ----- | ----- | ----- | ----- |
+| `space` | `x` | Todo | `TODO` |  |
+| `x` | `space` | Done | `DONE` |  |
+| `/` | `x` | In Progress | `IN_PROGRESS` |  |
+| `-` | `space` | Cancelled | `CANCELLED` |  |
diff --git a/tests/StatusSettingsReport.test.ts b/tests/StatusSettingsReport.test.ts
@@ -1,11 +1,34 @@
 import { StatusSettings } from '../src/Config/StatusSettings';
 import { tabulateStatusSettings } from '../src/StatusSettingsReport';
+import type { StatusCollection } from '../src/StatusCollection';
 import { verifyWithFileExtension } from './TestingTools/ApprovalTestHelpers';
+import { coreStatusesData, createStatuses } from './TestingTools/StatusesTestHelpers';
 
 describe('StatusSettingsReport', () => {
     it('should tabulate StatusSettings', () => {
         const statusSettings = new StatusSettings();
         const markdown = tabulateStatusSettings(statusSettings);
         verifyWithFileExtension(markdown, '.md');
     });
+
+    it('should include problems in table', () => {
+        const customStatusesData: StatusCollection = [
+            ['/', 'In Progress', 'x', 'IN_PROGRESS'],
+            ['/', 'In Progress DUPLICATE', 'x', 'IN_PROGRESS'],
+            ['X', 'X - conventionally DONE, but this is CANCELLED', ' ', 'CANCELLED'],
+            ['', '', '', 'TODO'], // A new, unedited status
+            ['p', 'Unknown next symbol', 'q', 'TODO'],
+            ['c', 'Followed by d', 'd', 'TODO'],
+            ['n', 'Non-task', 'n', 'NON_TASK'],
+            ['1', 'DONE followed by TODO', ' ', 'DONE'],
+            ['2', 'DONE followed by IN_PROGRESS', '/', 'DONE'],
+            ['3', 'DONE followed by DONE', 'x', 'DONE'],
+            ['4', 'DONE followed by CANCELLED', 'X', 'DONE'],
+            ['5', 'DONE followed by NON_TASK', 'n', 'DONE'],
+        ];
+        const { statusSettings } = createStatuses(coreStatusesData, customStatusesData);
+
+        const markdown = tabulateStatusSettings(statusSettings);
+        verifyWithFileExtension(markdown, '.md');
+    });
 });