Repository: obsidian-tasks-group/obsidian-tasks
Pull request #3020 (2024-08-09 13:19:56)

fix: Various improvements to cursor positions in Auto-Suggest
# Types of changes

Changes visible to users:

- [x] **Bug fix** (prefix: `fix` - non-breaking change which fixes an issue)

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)

## Description

Fix some unreported inconsistencies in AutoSuggest behaviour, discovered whilst adding support for auto-suggest on a new field.

- fix: Better dataview space & cursor in recurrence suggestions
- fix: Remove the need to type a space after 'id' and 'depends on' selected
- fix: Better dataview space & cursor in 'generate unique id' result

See the "Screenshots" section below for demos of each change.

## Motivation and Context

I want to extract some helper functions in `Suggestor.ts` as it is so repetitive - in doing so, I found some inconsistencies in the code, which explain some inconsistencies in the behaviour.

## How has this been tested?

- Automated tests
- Exploratory testing

## Screenshots (if appropriate)

In the following text, the `|` represents the cursor position after the change.

### Before

#### Tasks Format

```
- [ ] #task Tasks Format - Task 1 🔁 every day |
- [ ] #task Tasks Format - Task 2 🆔|
- [ ] #task Tasks Format - Task 3 ⛔|
- [ ] #task Tasks Format - Task 4 🆔 ousy9q| 
```

#### Dataview Format

```
- [ ] #task Dataview Format - Task 1 [repeat:: every day |]
- [ ] #task Dataview Format - Task 2 [id::|]
- [ ] #task Dataview Format - Task 3 [dependsOn::|]
- [ ] #task Dataview Format - Task 4 [id:: ousy9q| ]
```

### After

#### Tasks Format

```
- [ ] #task Tasks Format - Task 1 🔁 every day |
- [ ] #task Tasks Format - Task 2 🆔 |
- [ ] #task Tasks Format - Task 3 ⛔ |
- [ ] #task Tasks Format - Task 4 🆔 ousy9q |
```

#### Dataview Format

```
- [ ] #task Dataview Format - Task 1 [repeat:: every day] |
- [ ] #task Dataview Format - Task 2 [id:: |]
- [ ] #task Dataview Format - Task 3 [dependsOn:: |]
- [ ] #task Dataview Format - Task 4 [id:: ousy9q] |
```

## Checklist

- [x] My code follows the code style of this project and passes `yarn run lint`.
- [x] My change requires a change to the documentation.
- [x] I have [updated the documentation](https://publish.obsidian.md/tasks-contributing/Documentation/About+Documentation) accordingly.
- [x] My change has adequate [Unit Test coverage](https://publish.obsidian.md/tasks-contributing/Testing/About+Testing).

## Terms

- [x] My contribution follows 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/docs/Editing/Auto-Suggest.md b/docs/Editing/Auto-Suggest.md
index ab43c1af..fc716537 100644
--- a/docs/Editing/Auto-Suggest.md
+++ b/docs/Editing/Auto-Suggest.md
@@ -240,8 +240,8 @@ Similarly, you can type some fraction of the word `start` (of whatever length is
 | ⏬ lowest priority | ⏬  |
 | 🔁 recurring (repeat) | 🔁  |
 | ➕ created today (2022-07-11) | ➕ 2022-07-11  |
-| 🆔 id | 🆔 |
-| ⛔ depends on id | ⛔ |
+| 🆔 id | 🆔  |
+| ⛔ depends on id | ⛔  |
 | every | 🔁 every  |
 | every day | 🔁 every day  |
 | every week | 🔁 every week  |
@@ -291,7 +291,7 @@ Similarly, you can type some fraction of the word `start` (of whatever length is
 | next week (2022-07-18) | 🛫 2022-07-18  |
 | next month (2022-08-11) | 🛫 2022-08-11  |
 | next year (2023-07-11) | 🛫 2023-07-11  |
-| generate unique id | 🆔 ****** |
+| generate unique id | 🆔 ******  |
 <!-- endInclude -->
 
 ### How can I use auto-suggest features from other plugins together with the Tasks auto-suggest?
diff --git a/src/Suggestor/Suggestor.ts b/src/Suggestor/Suggestor.ts
index 1cfce3f7..0bbef5bf 100644
--- a/src/Suggestor/Suggestor.ts
+++ b/src/Suggestor/Suggestor.ts
@@ -68,7 +68,9 @@ export function makeDefaultSuggestionBuilder(
 
         // add Auto ID suggestions
         if (includeDependencySuggestions(canSaveEdits)) {
-            suggestions = suggestions.concat(addIDSuggestion(line, cursorPos, symbols.idSymbol, allTasks));
+            suggestions = suggestions.concat(
+                addIDSuggestion(line, cursorPos, symbols.idSymbol, dataviewMode, allTasks),
+            );
 
             // add dependecy suggestions
             suggestions = suggestions.concat(
@@ -244,18 +246,8 @@ function addDependencySuggestions(
     canSaveEdits: boolean,
 ) {
     if (includeDependencySuggestions(canSaveEdits)) {
-        // These don't reuse addField() because they don't have a space at the end of the appendText values.
-        if (!line.includes(symbols.idSymbol))
-            genericSuggestions.push({
-                displayText: `${symbols.idSymbol} id`,
-                appendText: `${symbols.idSymbol}`,
-            });
-
-        if (!line.includes(symbols.dependsOnSymbol))
-            genericSuggestions.push({
-                displayText: `${symbols.dependsOnSymbol} depends on id`,
-                appendText: `${symbols.dependsOnSymbol}`,
-            });
+        addField(genericSuggestions, line, symbols.idSymbol, 'id');
+        addField(genericSuggestions, line, symbols.dependsOnSymbol, 'depends on id');
     }
 }
 
@@ -380,13 +372,12 @@ function addDatesSuggestions(
         for (const match of genericMatches) {
             const parsedDate = DateParser.parseDate(match, true);
             const formattedDate = `${parsedDate.format(TaskRegularExpressions.dateFormat)}`;
-            const insertSkipValue = dataviewMode ? dateMatch[0].length + insertSkip : dateMatch[0].length;
             results.push({
                 suggestionType: 'match',
                 displayText: `${match} (${formattedDate})`,
                 appendText: `${datePrefix} ${formattedDate}` + postfix,
                 insertAt: dateMatch.index,
-                insertSkip: insertSkipValue,
+                insertSkip: calculateSkipValueForMatch(dataviewMode, insertSkip, dateMatch[0]),
             });
         }
     }
@@ -446,15 +437,12 @@ function addRecurrenceValueSuggestions(
             })?.toText();
             if (parsedRecurrence) {
                 const appendedText = `${recurrencePrefix} ${parsedRecurrence}` + postfix;
-                const insertSkipValue = dataviewMode
-                    ? recurrenceMatch[0].length + insertSkip
-                    : recurrenceMatch[0].length;
                 results.push({
                     suggestionType: 'match',
                     displayText: `✅ ${parsedRecurrence}`,
                     appendText: appendedText,
                     insertAt: recurrenceMatch.index,
-                    insertSkip: insertSkipValue,
+                    insertSkip: calculateSkipValueForMatch(dataviewMode, insertSkip, recurrenceMatch[0]),
                 });
                 // If the full match includes a complete valid suggestion *ending with space*,
                 // don't suggest anything. The user is trying to continue to type something that is likely
@@ -491,9 +479,9 @@ function addRecurrenceValueSuggestions(
             results.push({
                 suggestionType: 'match',
                 displayText: `${match}`,
-                appendText: `${recurrencePrefix} ${match} `,
+                appendText: `${recurrencePrefix} ${match}` + postfix,
                 insertAt: recurrenceMatch.index,
-                insertSkip: recurrenceMatch[0].length,
+                insertSkip: calculateSkipValueForMatch(dataviewMode, insertSkip, recurrenceMatch[0]),
             });
         }
     }
@@ -501,7 +489,9 @@ function addRecurrenceValueSuggestions(
     return results;
 }
 
-function addIDSuggestion(line: string, cursorPos: number, idSymbol: string, allTasks: Task[]) {
+function addIDSuggestion(line: string, cursorPos: number, idSymbol: string, dataviewMode: boolean, allTasks: Task[]) {
+    const { postfix, insertSkip } = getAdjusters(dataviewMode, line, cursorPos);
+
     const results: SuggestInfo[] = [];
     const idRegex = new RegExp(`(${idSymbol})\\s*(${taskIdRegex.source})?`, 'ug');
     const idMatch = matchIfCursorInRegex(line, idRegex, cursorPos);
@@ -511,9 +501,9 @@ function addIDSuggestion(line: string, cursorPos: number, idSymbol: string, allT
         results.push({
             suggestionType: 'match',
             displayText: 'generate unique id',
-            appendText: `${idSymbol} ${ID}`,
+            appendText: `${idSymbol} ${ID}` + postfix,
             insertAt: idMatch.index,
-            insertSkip: idSymbol.length,
+            insertSkip: calculateSkipValueForMatch(dataviewMode, insertSkip, idMatch[0]),
         });
     }
 
@@ -775,3 +765,7 @@ function cursorIsInTaskLineDescription(line: string, cursorPosition: number) {
 
     return cursorPosition >= beforeDescription.length;
 }
+
+function calculateSkipValueForMatch(dataviewMode: boolean, insertSkip: number, match: string) {
+    return dataviewMode ? match.length + insertSkip : match.length;
+}


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
index 0fb49d41..88230c45 100644
--- 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
@@ -48,10 +48,10 @@
   },
   {
     "displayText": "id:: id",
-    "appendText": "id::"
+    "appendText": "id:: "
   },
   {
     "displayText": "dependsOn:: depends on id",
-    "appendText": "dependsOn::"
+    "appendText": "dependsOn:: "
   }
 ]
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 f1182b13..11543075 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
@@ -10,21 +10,21 @@
 | priority:: lowest priority | priority:: lowest]  |
 | repeat:: recurring (repeat) | repeat::  |
 | created:: created today (2022-07-11) | created:: 2022-07-11]  |
-| id:: id | id:: |
-| dependsOn:: depends on id | dependsOn:: |
-| every | repeat:: every  |
-| every day | repeat:: every day  |
-| every week | repeat:: every week  |
-| every month | repeat:: every month  |
-| every month on the | repeat:: every month on the  |
-| every year | repeat:: every year  |
-| every week on Sunday | repeat:: every week on Sunday  |
-| every week on Monday | repeat:: every week on Monday  |
-| every week on Tuesday | repeat:: every week on Tuesday  |
-| every week on Wednesday | repeat:: every week on Wednesday  |
-| 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  |
+| id:: id | id::  |
+| dependsOn:: depends on id | dependsOn::  |
+| every | repeat:: every]  |
+| every day | repeat:: every day]  |
+| every week | repeat:: every week]  |
+| every month | repeat:: every month]  |
+| every month on the | repeat:: every month on the]  |
+| every year | repeat:: every year]  |
+| every week on Sunday | repeat:: every week on Sunday]  |
+| every week on Monday | repeat:: every week on Monday]  |
+| every week on Tuesday | repeat:: every week on Tuesday]  |
+| every week on Wednesday | repeat:: every week on Wednesday]  |
+| 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]  |
@@ -61,4 +61,4 @@
 | 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]  |
-| generate unique id | id:: ****** |
+| generate unique id | id:: ******]  |
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
index 67d0c6d2..1c0567b9 100644
--- 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
@@ -47,10 +47,10 @@
   },
   {
     "displayText": "🆔 id",
-    "appendText": "🆔"
+    "appendText": "🆔 "
   },
   {
     "displayText": "⛔ depends on id",
-    "appendText": "⛔"
+    "appendText": "⛔ "
   }
 ]
diff --git a/tests/Suggestor/Suggestor.test.auto-complete_with__emoji__symbols_show_all_suggested_text.approved.md b/tests/Suggestor/Suggestor.test.auto-complete_with__emoji__symbols_show_all_suggested_text.approved.md
index 3463a71a..0b42b9d6 100644
--- a/tests/Suggestor/Suggestor.test.auto-complete_with__emoji__symbols_show_all_suggested_text.approved.md
+++ b/tests/Suggestor/Suggestor.test.auto-complete_with__emoji__symbols_show_all_suggested_text.approved.md
@@ -11,8 +11,8 @@
 | ⏬ lowest priority | ⏬  |
 | 🔁 recurring (repeat) | 🔁  |
 | ➕ created today (2022-07-11) | ➕ 2022-07-11  |
-| 🆔 id | 🆔 |
-| ⛔ depends on id | ⛔ |
+| 🆔 id | 🆔  |
+| ⛔ depends on id | ⛔  |
 | every | 🔁 every  |
 | every day | 🔁 every day  |
 | every week | 🔁 every week  |
@@ -62,4 +62,4 @@
 | next week (2022-07-18) | 🛫 2022-07-18  |
 | next month (2022-08-11) | 🛫 2022-08-11  |
 | next year (2023-07-11) | 🛫 2023-07-11  |
-| generate unique id | 🆔 ****** |
+| generate unique id | 🆔 ******  |
diff --git a/tests/Suggestor/Suggestor.test.ts b/tests/Suggestor/Suggestor.test.ts
index 95a49426..5069d651 100644
--- a/tests/Suggestor/Suggestor.test.ts
+++ b/tests/Suggestor/Suggestor.test.ts
@@ -61,7 +61,7 @@ function cursorPosition(line: string): [lineWithoutCursor: string, cursorIndex:
  * @returns the SuggestInfo, with all ID's Masked
  */
 function maskIDSuggestionForTesting(idSymbol: string, suggestions: SuggestInfo[]): SuggestInfo[] {
-    const idRegex = new RegExp(`${idSymbol}( [0-9a-zA-Z]*)`, 'ug');
+    const idRegex = new RegExp(`${idSymbol}( [0-9a-zA-Z]+)`, 'ug');
     suggestions.forEach((element) => {
         element.appendText = element.appendText.replace(idRegex, `${idSymbol} ******`);
     });