Repository: obsidian-tasks-group/obsidian-tasks
Pull request #2675 (2024-02-24 14:22:19)

refactor: add tests and refactor auto suggest
# Description

Done by pairing with @claremacrae 

## Motivation and Context

- Make the behaviour of the code visible in tests.
- Remove dependence of code on a global setting.
- Reduce repetition in code

## How has this been tested?

Unit tests + exploratory testing.


## Types of changes


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 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/src/Config/Settings.ts b/src/Config/Settings.ts
index 89b8022c..ea4282d5 100644
--- a/src/Config/Settings.ts
+++ b/src/Config/Settings.ts
@@ -42,13 +42,13 @@ export const TASK_FORMATS = {
     tasksPluginEmoji: {
         displayName: 'Tasks Emoji Format',
         taskSerializer: new DefaultTaskSerializer(DEFAULT_SYMBOLS),
-        buildSuggestions: makeDefaultSuggestionBuilder(DEFAULT_SYMBOLS, DEFAULT_MAX_GENERIC_SUGGESTIONS),
+        buildSuggestions: makeDefaultSuggestionBuilder(DEFAULT_SYMBOLS, DEFAULT_MAX_GENERIC_SUGGESTIONS, false),
     },
     dataview: {
         displayName: 'Dataview',
         taskSerializer: new DataviewTaskSerializer(),
         buildSuggestions: onlySuggestIfBracketOpen(
-            makeDefaultSuggestionBuilder(DATAVIEW_SYMBOLS, DEFAULT_MAX_GENERIC_SUGGESTIONS),
+            makeDefaultSuggestionBuilder(DATAVIEW_SYMBOLS, DEFAULT_MAX_GENERIC_SUGGESTIONS, true),
             [
                 ['(', ')'],
                 ['[', ']'],
diff --git a/src/Suggestor/Suggestor.ts b/src/Suggestor/Suggestor.ts
index 2def1e7b..1fc1e50d 100644
--- a/src/Suggestor/Suggestor.ts
+++ b/src/Suggestor/Suggestor.ts
@@ -17,6 +17,7 @@ export const DEFAULT_MAX_GENERIC_SUGGESTIONS = 5;
 export function makeDefaultSuggestionBuilder(
     symbols: DefaultTaskSerializerSymbols,
     maxGenericSuggestions: number /** See {@link DEFAULT_MAX_GENERIC_SUGGESTIONS} */,
+    dataviewMode: boolean,
 ): SuggestionBuilder {
     // NEW_TASK_FIELD_EDIT_REQUIRED
     const datePrefixRegex = [symbols.startDateSymbol, symbols.scheduledDateSymbol, symbols.dueDateSymbol].join('|');
@@ -28,21 +29,23 @@ export function makeDefaultSuggestionBuilder(
 
         // Step 1: add date suggestions if relevant
         suggestions = suggestions.concat(
-            addDatesSuggestions(line, cursorPos, settings, datePrefixRegex, maxGenericSuggestions),
+            addDatesSuggestions(line, cursorPos, settings, datePrefixRegex, maxGenericSuggestions, dataviewMode),
         );
 
         // Step 2: add recurrence suggestions if relevant
-        suggestions = suggestions.concat(addRecurrenceSuggestions(line, cursorPos, settings, symbols.recurrenceSymbol));
+        suggestions = suggestions.concat(
+            addRecurrenceSuggestions(line, cursorPos, settings, symbols.recurrenceSymbol, dataviewMode),
+        );
 
         // Step 3: add task property suggestions ('due', 'recurrence' etc)
-        suggestions = suggestions.concat(addTaskPropertySuggestions(line, cursorPos, settings, symbols));
+        suggestions = suggestions.concat(addTaskPropertySuggestions(line, cursorPos, settings, symbols, dataviewMode));
 
         // Unless we have a suggestion that is a match for something the user is currently typing, add
         // an 'Enter' entry in the beginning of the menu, so an Enter press will move to the next line
         // rather than insert a suggestion
         if (suggestions.length > 0 && !suggestions.some((value) => value.suggestionType === 'match')) {
             // No actual match, only default ones
-            if (settings.taskFormat != 'dataview') {
+            if (!dataviewMode) {
                 suggestions.unshift({
                     suggestionType: 'empty',
                     displayText: '⏎',
@@ -58,6 +61,20 @@ export function makeDefaultSuggestionBuilder(
     };
 }
 
+function getAdjusters(dataviewMode: boolean, line: string, cursorPos: number) {
+    const closingBracket =
+        lastOpenBracket(line.substring(0, cursorPos), [
+            // TODO this array duplicates code in Settings.ts. Can we introduce an abstraction for this?
+            ['(', ')'],
+            ['[', ']'],
+        ]) == '('
+            ? ')'
+            : ']';
+    const postfix = dataviewMode ? closingBracket + ' ' : ' ';
+    const insertSkip = dataviewMode && line.length > cursorPos && line.charAt(cursorPos) === closingBracket ? 1 : 0;
+    return { postfix, insertSkip };
+}
+
 /*
  * Get suggestions for generic task components, e.g. a priority or a 'due' symbol
  */
@@ -66,21 +83,13 @@ function addTaskPropertySuggestions(
     cursorPos: number,
     _settings: Settings,
     symbols: DefaultTaskSerializerSymbols,
+    dataviewMode: boolean,
 ): SuggestInfo[] {
     const hasPriority = (line: string) =>
         Object.values(symbols.prioritySymbols).some((value) => value.length > 0 && line.includes(value));
 
     const genericSuggestions: SuggestInfo[] = [];
-    const dataviewMode = _settings.taskFormat === 'dataview';
-    const close_bracket =
-        lastOpenBracket(line.substring(0, cursorPos), [
-            ['(', ')'],
-            ['[', ']'],
-        ]) == '('
-            ? ')'
-            : ']';
-    const postfix = dataviewMode ? close_bracket + ' ' : ' ';
-    const insertSkip = dataviewMode && line.length > cursorPos && line.charAt(cursorPos) === close_bracket ? 1 : 0;
+    const { postfix, insertSkip } = getAdjusters(dataviewMode, line, cursorPos);
 
     // NEW_TASK_FIELD_EDIT_REQUIRED
     if (!line.includes(symbols.dueDateSymbol))
@@ -107,10 +116,9 @@ function addTaskPropertySuggestions(
             const prioritySymbol = prioritySymbols[priorityText];
 
             genericSuggestions.push({
-                displayText:
-                    _settings.taskFormat == 'dataview'
-                        ? `${prioritySymbol} priority`
-                        : `${prioritySymbol} ${priorityText.toLowerCase()} priority`,
+                displayText: dataviewMode
+                    ? `${prioritySymbol} priority`
+                    : `${prioritySymbol} ${priorityText.toLowerCase()} priority`,
                 appendText: `${prioritySymbol}${postfix}`,
                 insertSkip: dataviewMode ? insertSkip : undefined,
             });
@@ -184,6 +192,7 @@ function addDatesSuggestions(
     settings: Settings,
     datePrefixRegex: string,
     maxGenericSuggestions: number,
+    dataviewMode: boolean,
 ): SuggestInfo[] {
     const genericSuggestions = [
         'today',
@@ -200,16 +209,7 @@ function addDatesSuggestions(
         'next year',
     ];
 
-    const dataviewMode = settings.taskFormat === 'dataview';
-    const close_bracket =
-        lastOpenBracket(line.substring(0, cursorPos), [
-            ['(', ')'],
-            ['[', ']'],
-        ]) == '('
-            ? ')'
-            : ']';
-    const postfix = dataviewMode ? close_bracket + ' ' : ' ';
-    const insertSkip = dataviewMode && line.length > cursorPos && line.charAt(cursorPos) === close_bracket ? 1 : 0;
+    const { postfix, insertSkip } = getAdjusters(dataviewMode, line, cursorPos);
 
     const results: SuggestInfo[] = [];
     const dateRegex = new RegExp(`(${datePrefixRegex})\\s*([0-9a-zA-Z ]*)`, 'ug');
@@ -280,7 +280,13 @@ function addDatesSuggestions(
  * Generic predefined suggestions, in turn, also have two options: either filtered (if the user started typing
  * something where a recurrence is expected) or unfiltered
  */
-function addRecurrenceSuggestions(line: string, cursorPos: number, settings: Settings, recurrenceSymbol: string) {
+function addRecurrenceSuggestions(
+    line: string,
+    cursorPos: number,
+    settings: Settings,
+    recurrenceSymbol: string,
+    dataviewMode: boolean,
+) {
     const genericSuggestions = [
         'every',
         'every day',
@@ -296,16 +302,8 @@ function addRecurrenceSuggestions(line: string, cursorPos: number, settings: Set
         'every week on Friday',
         'every week on Saturday',
     ];
-    const dataviewMode = settings.taskFormat === 'dataview';
-    const close_bracket =
-        lastOpenBracket(line.substring(0, cursorPos), [
-            ['(', ')'],
-            ['[', ']'],
-        ]) == '('
-            ? ')'
-            : ']';
-    const postfix = dataviewMode ? close_bracket + ' ' : ' ';
-    const insertSkip = dataviewMode && line.length > cursorPos && line.charAt(cursorPos) === close_bracket ? 1 : 0;
+
+    const { postfix, insertSkip } = getAdjusters(dataviewMode, line, cursorPos);
 
     const results: SuggestInfo[] = [];
     const recurrenceRegex = new RegExp(`(${recurrenceSymbol})\\s*([0-9a-zA-Z ]*)`, 'ug');


Test patch:
diff --git a/tests/Suggestor/Suggestor.test.auto-complete_with__dataview__symbols_offers_basic_completion_options_for_an_empty_task.approved.json b/tests/Suggestor/Suggestor.test.auto-complete_with__dataview__symbols_offers_basic_completion_options_for_an_empty_task.approved.json
new file mode 100644
index 00000000..dea2870e
--- /dev/null
+++ b/tests/Suggestor/Suggestor.test.auto-complete_with__dataview__symbols_offers_basic_completion_options_for_an_empty_task.approved.json
@@ -0,0 +1,29 @@
+[
+  {
+    "displayText": "due:: due date",
+    "appendText": "due:: "
+  },
+  {
+    "displayText": "start:: start date",
+    "appendText": "start:: "
+  },
+  {
+    "displayText": "scheduled:: scheduled date",
+    "appendText": "scheduled:: "
+  },
+  {
+    "displayText": "priority:: high priority",
+    "appendText": "priority:: high] ",
+    "insertSkip": 0
+  },
+  {
+    "displayText": "priority:: medium priority",
+    "appendText": "priority:: medium] ",
+    "insertSkip": 0
+  },
+  {
+    "displayText": "priority:: low priority",
+    "appendText": "priority:: low] ",
+    "insertSkip": 0
+  }
+]
diff --git a/tests/Suggestor/Suggestor.test.auto-complete_with__dataview__symbols_show_all_suggested_text.approved.md b/tests/Suggestor/Suggestor.test.auto-complete_with__dataview__symbols_show_all_suggested_text.approved.md
index 95fac53c..dc1c5460 100644
--- a/tests/Suggestor/Suggestor.test.auto-complete_with__dataview__symbols_show_all_suggested_text.approved.md
+++ b/tests/Suggestor/Suggestor.test.auto-complete_with__dataview__symbols_show_all_suggested_text.approved.md
@@ -1,16 +1,15 @@
 | suggestion | expansion |
 | ----- | ----- |
-| ⏎ | &lt;new line> |
 | due:: due date | due::  |
 | start:: start date | start::  |
 | scheduled:: scheduled date | scheduled::  |
-| priority:: high high priority | priority:: high  |
-| priority:: medium medium priority | priority:: medium  |
-| priority:: low low priority | priority:: low  |
-| priority:: highest highest priority | priority:: highest  |
-| priority:: lowest lowest priority | priority:: lowest  |
+| priority:: high priority | priority:: high]  |
+| priority:: medium priority | priority:: medium]  |
+| priority:: low priority | priority:: low]  |
+| priority:: highest priority | priority:: highest]  |
+| priority:: lowest priority | priority:: lowest]  |
 | repeat:: recurring (repeat) | repeat::  |
-| created:: created today (2022-07-11) | created:: 2022-07-11  |
+| created:: created today (2022-07-11) | created:: 2022-07-11]  |
 | every | repeat:: every  |
 | every day | repeat:: every day  |
 | every week | repeat:: every week  |
@@ -24,39 +23,39 @@
 | every week on Thursday | repeat:: every week on Thursday  |
 | every week on Friday | repeat:: every week on Friday  |
 | every week on Saturday | repeat:: every week on Saturday  |
-| today (2022-07-11) | due:: 2022-07-11  |
-| tomorrow (2022-07-12) | due:: 2022-07-12  |
-| Sunday (2022-07-17) | due:: 2022-07-17  |
-| Monday (2022-07-18) | due:: 2022-07-18  |
-| Tuesday (2022-07-12) | due:: 2022-07-12  |
-| Wednesday (2022-07-13) | due:: 2022-07-13  |
-| Thursday (2022-07-14) | due:: 2022-07-14  |
-| Friday (2022-07-15) | due:: 2022-07-15  |
-| Saturday (2022-07-16) | due:: 2022-07-16  |
-| next week (2022-07-18) | due:: 2022-07-18  |
-| next month (2022-08-11) | due:: 2022-08-11  |
-| next year (2023-07-11) | due:: 2023-07-11  |
-| today (2022-07-11) | scheduled:: 2022-07-11  |
-| tomorrow (2022-07-12) | scheduled:: 2022-07-12  |
-| Sunday (2022-07-17) | scheduled:: 2022-07-17  |
-| Monday (2022-07-18) | scheduled:: 2022-07-18  |
-| Tuesday (2022-07-12) | scheduled:: 2022-07-12  |
-| Wednesday (2022-07-13) | scheduled:: 2022-07-13  |
-| Thursday (2022-07-14) | scheduled:: 2022-07-14  |
-| Friday (2022-07-15) | scheduled:: 2022-07-15  |
-| Saturday (2022-07-16) | scheduled:: 2022-07-16  |
-| next week (2022-07-18) | scheduled:: 2022-07-18  |
-| next month (2022-08-11) | scheduled:: 2022-08-11  |
-| next year (2023-07-11) | scheduled:: 2023-07-11  |
-| today (2022-07-11) | start:: 2022-07-11  |
-| tomorrow (2022-07-12) | start:: 2022-07-12  |
-| Sunday (2022-07-17) | start:: 2022-07-17  |
-| Monday (2022-07-18) | start:: 2022-07-18  |
-| Tuesday (2022-07-12) | start:: 2022-07-12  |
-| Wednesday (2022-07-13) | start:: 2022-07-13  |
-| Thursday (2022-07-14) | start:: 2022-07-14  |
-| Friday (2022-07-15) | start:: 2022-07-15  |
-| Saturday (2022-07-16) | start:: 2022-07-16  |
-| next week (2022-07-18) | start:: 2022-07-18  |
-| next month (2022-08-11) | start:: 2022-08-11  |
-| next year (2023-07-11) | start:: 2023-07-11  |
+| today (2022-07-11) | due:: 2022-07-11]  |
+| tomorrow (2022-07-12) | due:: 2022-07-12]  |
+| Sunday (2022-07-17) | due:: 2022-07-17]  |
+| Monday (2022-07-18) | due:: 2022-07-18]  |
+| Tuesday (2022-07-12) | due:: 2022-07-12]  |
+| Wednesday (2022-07-13) | due:: 2022-07-13]  |
+| Thursday (2022-07-14) | due:: 2022-07-14]  |
+| Friday (2022-07-15) | due:: 2022-07-15]  |
+| Saturday (2022-07-16) | due:: 2022-07-16]  |
+| next week (2022-07-18) | due:: 2022-07-18]  |
+| next month (2022-08-11) | due:: 2022-08-11]  |
+| next year (2023-07-11) | due:: 2023-07-11]  |
+| today (2022-07-11) | scheduled:: 2022-07-11]  |
+| tomorrow (2022-07-12) | scheduled:: 2022-07-12]  |
+| Sunday (2022-07-17) | scheduled:: 2022-07-17]  |
+| Monday (2022-07-18) | scheduled:: 2022-07-18]  |
+| Tuesday (2022-07-12) | scheduled:: 2022-07-12]  |
+| Wednesday (2022-07-13) | scheduled:: 2022-07-13]  |
+| Thursday (2022-07-14) | scheduled:: 2022-07-14]  |
+| Friday (2022-07-15) | scheduled:: 2022-07-15]  |
+| Saturday (2022-07-16) | scheduled:: 2022-07-16]  |
+| next week (2022-07-18) | scheduled:: 2022-07-18]  |
+| next month (2022-08-11) | scheduled:: 2022-08-11]  |
+| next year (2023-07-11) | scheduled:: 2023-07-11]  |
+| today (2022-07-11) | start:: 2022-07-11]  |
+| tomorrow (2022-07-12) | start:: 2022-07-12]  |
+| Sunday (2022-07-17) | start:: 2022-07-17]  |
+| Monday (2022-07-18) | start:: 2022-07-18]  |
+| Tuesday (2022-07-12) | start:: 2022-07-12]  |
+| Wednesday (2022-07-13) | start:: 2022-07-13]  |
+| Thursday (2022-07-14) | start:: 2022-07-14]  |
+| Friday (2022-07-15) | start:: 2022-07-15]  |
+| Saturday (2022-07-16) | start:: 2022-07-16]  |
+| next week (2022-07-18) | start:: 2022-07-18]  |
+| next month (2022-08-11) | start:: 2022-08-11]  |
+| next year (2023-07-11) | start:: 2023-07-11]  |
diff --git a/tests/Suggestor/Suggestor.test.auto-complete_with__emoji__symbols_offers_basic_completion_options_for_an_empty_task.approved.json b/tests/Suggestor/Suggestor.test.auto-complete_with__emoji__symbols_offers_basic_completion_options_for_an_empty_task.approved.json
new file mode 100644
index 00000000..10fe4d6b
--- /dev/null
+++ b/tests/Suggestor/Suggestor.test.auto-complete_with__emoji__symbols_offers_basic_completion_options_for_an_empty_task.approved.json
@@ -0,0 +1,27 @@
+[
+  {
+    "suggestionType": "empty",
+    "displayText": "⏎",
+    "appendText": "\n"
+  },
+  {
+    "displayText": "📅 due date",
+    "appendText": "📅 "
+  },
+  {
+    "displayText": "🛫 start date",
+    "appendText": "🛫 "
+  },
+  {
+    "displayText": "⏳ scheduled date",
+    "appendText": "⏳ "
+  },
+  {
+    "displayText": "⏫ high priority",
+    "appendText": "⏫ "
+  },
+  {
+    "displayText": "🔼 medium priority",
+    "appendText": "🔼 "
+  }
+]
diff --git a/tests/Suggestor/Suggestor.test.ts b/tests/Suggestor/Suggestor.test.ts
index b01dacfd..7a7dadf6 100644
--- a/tests/Suggestor/Suggestor.test.ts
+++ b/tests/Suggestor/Suggestor.test.ts
@@ -1,6 +1,7 @@
 /**
  * @jest-environment jsdom
  */
+import { verifyAsJson } from 'approvals/lib/Providers/Jest/JestApprovals';
 import moment from 'moment';
 import * as chrono from 'chrono-node';
 import { getSettings } from '../../src/Config/Settings';
@@ -54,29 +55,20 @@ const MAX_GENERIC_SUGGESTIONS_FOR_TESTS = 50;
 describe.each([
     { name: 'emoji', symbols: DEFAULT_SYMBOLS },
     { name: 'dataview', symbols: DATAVIEW_SYMBOLS },
-])("auto-complete with '$name' symbols", ({ symbols }) => {
-    const buildSuggestions = makeDefaultSuggestionBuilder(symbols, MAX_GENERIC_SUGGESTIONS_FOR_TESTS);
-    const {
-        dueDateSymbol,
-        scheduledDateSymbol,
-        startDateSymbol,
-        createdDateSymbol,
-        recurrenceSymbol,
-        prioritySymbols,
-    } = symbols;
+])("auto-complete with '$name' symbols", ({ name, symbols }) => {
+    const buildSuggestions = makeDefaultSuggestionBuilder(
+        symbols,
+        MAX_GENERIC_SUGGESTIONS_FOR_TESTS,
+        name === 'dataview',
+    );
+
+    const { dueDateSymbol, scheduledDateSymbol, startDateSymbol, createdDateSymbol, recurrenceSymbol } = symbols;
     it('offers basic completion options for an empty task', () => {
         // Arrange
         const originalSettings = getSettings();
         const line = '- [ ] ';
         const suggestions: SuggestInfo[] = buildSuggestions(line, 5, originalSettings);
-        expect(suggestions).toEqual([
-            { suggestionType: 'empty', displayText: '⏎', appendText: '\n' },
-            { displayText: `${dueDateSymbol} due date`, appendText: `${dueDateSymbol} ` },
-            { displayText: `${startDateSymbol} start date`, appendText: `${startDateSymbol} ` },
-            { displayText: `${scheduledDateSymbol} scheduled date`, appendText: `${scheduledDateSymbol} ` },
-            { displayText: `${prioritySymbols.High} high priority`, appendText: `${prioritySymbols.High} ` },
-            { displayText: `${prioritySymbols.Medium} medium priority`, appendText: `${prioritySymbols.Medium} ` },
-        ]);
+        verifyAsJson(suggestions);
     });
 
     it('offers generic due date completions', () => {
@@ -140,7 +132,17 @@ describe.each([
 
         line = '- [ ] some task tod';
         suggestions = buildSuggestions(line, 19, originalSettings);
-        expect(suggestions[0].suggestionType).toEqual('empty');
+        if (name === 'emoji') {
+            // The first suggestion is new line
+            expect(suggestions[0].suggestionType).toEqual('empty');
+        } else if (name === 'dataview') {
+            // the new line suggestion is not offered for dataview
+            expect(suggestions[0].suggestionType).toEqual(undefined);
+        } else {
+            // we should never reach here
+            // add a new case above if adding a new format
+            expect(1).toEqual(2);
+        }
         expect(suggestions[0].displayText).not.toContain('created today');
     });