feat: Add Auto Suggest for Task dependencies

[Genei180]

Description

When a task is edited Auto Suggest, now Allows for Adding of ID's and Blocked By ID's.

If a task that blocks another is Choosen via the Auto Suggest, the ID gets Automatically Appended.

Motivation and Context

Fixes #2681

How has this been tested?

Simple Test Case were appended.
ID's need to be masked, because their Creation is Random via UUID Algorithm and change Every Time they get suggested.

Please, recheck, but in my understanding only the ordering changed for some reason.

Screenshots (if appropriate)

Types of changes

Changes visible to users:

• Bug fix (prefix: fix - non-breaking change which fixes an issue)
• New feature (prefix: feat - non-breaking change which adds functionality)
• Breaking change (prefix: feat!! or fix!! - fix or feature that would cause existing functionality to not work as expected)
• Documentation (prefix: docs - improvements to any documentation content for users)
• Sample vault (prefix: vault - improvements to the Tasks-Demo sample vault)
• Contributing Guidelines (prefix: contrib - any improvements to documentation content for contributors - see Contributing to Tasks)

Internal changes:

• Refactor (prefix: refactor - non-breaking change which only improves the design or structure of existing code, and making no changes to its external behaviour)
• Tests (prefix: test - additions and improvements to unit tests and the smoke tests)
• Infrastructure (prefix: chore - examples include GitHub Actions, issue templates)

Checklist

• 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.
  -> Should have please recheck.

Terms

• My contribution follow this project's contributing guide
• I agree to follow this project's Code of Conduct

[claremacrae]

Hi @Genei180, thank you very much indeed for working on this.

Exploratory testing - and what the behaviour is...

I'm unsure from the screenshot exactly how what the intended behaviour of the new code is, or what kinds of scenarios you have tested it with.

Thinking about the kinds of things that would need be covered by exploratory testing, can you kindly record the things that you have done to try and break it - what kind of corner cases and different situations have you tried to break it with, in Obsidian?

Documentation

Can you at least note here in the ticket a first draft of the bullet points that would need to be covered in the documentation of this new feature?

My workload

I am sad to say that I am beyond my PR capacity at the moment, with two wide-ranging PRs already open (reminders, and relations between tasks), so I will not be able to give this a thorough review as quickly as I would have liked.

I made a few notes on my first read through the code, though, and I'll add those as a review now, in the hope that you'll be able to answer them whilst this is fresh in your memory...

Initial observations from a quick play with it...

How does it determine the order of tasks in the menu?

Later: Ohhhh - it defaults to adding a dependency on the previous task, which is really nice!

It can create repeated dependencies:

Which confuses the Edit Task modal:

I was wondering how it would cope with adding dependencies on tasks in other files

... but it seems that it only offers tasks in the file being edited.

I am comfortable with this, as it is a significant simplification - but one that is worth documenting.

Names of the fields - should be 'depends on', not 'blocked by'

I just realised that calling it 'blocked by' is a bit misleading... the field name is 'depends on'.

Please see Search concepts ...

[claremacrae]

A few thoughts after an initial read-through...

[claremacrae]

@Genei180 This is going to be a HUGE usability improvement to the Task Dependencies feature - thanks very much indeed for this work!!

[Genei180]

try and break it - what kind of corner cases and different situations have you tried to break it with, in Obsidian?

To be Honest, I'm not experienced in that Field, so non for now.

Can you at least note here in the ticket a first draft of the bullet points that would need to be covered in the documentation of this new feature?

The Feature, adds to the Auto Suggest, which in turn complements the Task Dependencies Feature.
Nothing Changed in the Task Dependencies Feature, so there should be no Doc Change needed for the Dependencies Feature.

The Auto Suggest should Help the User as Much as Possible, if he needs to look it up in the Documentation, I would say I did Something very wrong.
Additionally, it doesn't change any Behavior at all, except the UI/UX, which should be as Streamlined as Possible, and without Documentation in my Opinion. Should be Self Explanatory.

Could you point me in the right Direction, what needs documenting in your Opinion?

My workload

I am sad to say that I am beyond my PR capacity at the moment, with two wide-ranging PRs already open (reminders, and relations between tasks), so I will not be able to give this a thorough review as quickly as I would have liked.

Don't Worry, I'm out of Time all the Time too :)
A good Review will take its Time.
Please Take as much Time as you need.

Initial observations from a quick play with it...

How does it determine the order of tasks in the menu?

The Behavior is not from me.
It takes Tasks Closer related too it (In Same File, Closer to the Line etc...) first.
The Code for it is in "src\Suggestor\DependencyHelpers.ts" Line: 95.

results.sort((a, b) => {
            const aInSamePath = a.taskLocation.path === task.taskLocation.path;
            const bInSamePath = b.taskLocation.path === task.taskLocation.path;

            // prioritise tasks close to this task in the same file
            if (aInSamePath && bInSamePath) {
                return (
                    Math.abs(a.taskLocation.lineNumber - task.taskLocation.lineNumber) -
                    Math.abs(b.taskLocation.lineNumber - task.taskLocation.lineNumber)
                );
            } else if (aInSamePath) {
                return -1;
            } else if (bInSamePath) {
                return 1;
            } else {
                return 0;
            }
        });

It can create repeated dependencies:
Which confuses the Edit Task modal:

True, should be solved when I add, the Values mentioned by this Comment: #2771 (comment)

(Should be Introduced now)

I was wondering how it would cope with adding dependencies on tasks in other files
... but it seems that it only offers tasks in the file being edited.

For me, it shows tasks in other Files as Well, if i Type a Corresponding Task?
(I just discovered a Bug, where the Last Letter Typed, gets appended too the ID, when the Task is from another File.
Will Look into it)

I am comfortable with this, as it is a significant simplification - but one that is worth documenting.

Should not be this Way.

Names of the fields - should be 'depends on', not 'blocked by'

• Changed the Naming (Including Multiple Functions Calls)

[claremacrae]

Wow, thank you @Genei180 for some really helpful comments, which I look forward to reading and replying to when I can give them the time they deserve.

And especially, thank you for these kind comments, which I greatly appreciate.

Don't Worry, I'm out of Time all the Time too :)
A good Review will take its Time.
Please Take as much Time as you need.

[claremacrae]

@Genei180 - Huge thanks for yesterday's updates.

This is definitely good enough to merge the PR now.

There is one small code change that I want to make, and in the interests of saving time I'm going to try and make it and push it to your PR branch. This normally works, but if it doesn't work, I'll add more info here and ask you to make the edit....

[claremacrae]

Super functionality - thank you very much indeed for this!

[claremacrae]

Hi @Genei180,

I'm writing tests for the new behaviour, trying to get good coverage of all the behaviours, before fixing one or two bugs I found in testing this feature in my own main vault.

I'm having a bit of trouble understanding the intention of some bits of the regular expression on line 445 - can you remember the reasons for any of these bits:

• Why is (${dependsOnSymbol}) captured as a group?
• Why is a space character included in [0-9a-zA-Z ^,]?
• Why are the number of occurrences all * rather than + - in other words, why is 0 occurrences allowed throughout?

I'm asking because I want to do some refactoring to make use of an existing regex... below ... to specify the allowed characters in IDs, so first I need to add tests for all the current behaviours of that regular expression - which means understanding its requirements...

// The allowed characters in a single task id:
export const taskIdRegex = /[a-zA-Z0-9-_]+/;

Thanks in advance....

[Genei180]

Hi @claremacrae,

Will Happily Help.
A helpfull Resource can be regex101

It Provides Easy Visualisation (Left), Explanation to the Right (Tab Closed, Right), and Displays the Group Matches(Right).

I tried to make it as Verbose as Possible, with the Following Variable Assignments, too Explain what each Capture Group, does/Matches.

// dependsOnMatch[1] = Depends On Symbol
const existingDependsOnIdStrings = dependsOnMatch[2] || '';
const newTaskToAppend = dependsOnMatch[3];

Why is (${dependsOnSymbol}) captured as a group?

Good Question, and you're Right it could Probably be removed.
Used Originally, and then Replaced it with the Passed in Parameter.
So i forgot too Change the Regex, and it became redundant.

Why is a space character included in [0-9a-zA-Z ^,]?

It Catches any Spaces, that were put there for Formatting Reasons.
The Second Capture Group, captures, all Existing ID's, to re-add to the Suggestion.
You could Probably, remove it, and just adjust the insert At Index, to append it.
But i found it Useful, to match the Whole Depends on as Regex.

If the Space is not in the Second Regex Capture Group, you can't Distinguish Existing IDs, form Typed Text:

All is matched, in Capture Group 3, and you need too Split it up anyway.

In Other Words, it does not Match ID's, but all via ", " Chained ID's, till the last ",".
Everything after is Interpreted as Text too Search for in Suggested Tasks.

Why are the number of occurrences all * rather than + - in other words, why is 0 occurrences allowed throughout?

Because, the Suggestion needs to occur, after the DependsOn Symbol, even if nothing is present.

I'm asking because I want to do some refactoring to make use of an existing regex.

I guess the Motivation behind is readability. I understand that.
You could Probably Split up the Regex in Multiple Parts.
For my Part, i didn't find this more Readable or easier too Understand.
I tried solving that over the Variable Name, which explains what the Capture Group does.
This should guarantee short, easy too read Code.

If i could have written something more Clearly or better, please let me know so i can learn from it.

Additionally, I can assist you with testing, but I'm not Very Experienced what to Test for.
So I need to Input like, "Please, write Tests for the Regex Capture Group" or something similar, so I know where to start and what to Test for.

[claremacrae]

Hi, thank you for a very helpful reply indeed....

I'm thinking about a mixture of understanding the behaviour, testing it, and maintaining it.

Here's a bit of a brain dump.

tl;dr

1. the first thing to do is to fix the new test which is marked as .failing - see below...
2. sorry to be giving this feedback now rather than before merging... I normally try and do this level of testing before merging, but needed to have the new code in my vault to be able to test it with real data....
3. it might be easier if we had a video call about this rather than typing here - I have an email address in my GitHub profile if this is of interest to you...

Allowed characters in IDs in Tasks

The valid characters in IDs are documented in ID.

The valid characters in IDs are defined in a single source of truth:

obsidian-tasks/src/TaskSerializer/DefaultTaskSerializer.ts

Lines 48 to 49 in fd391f2

	// The allowed characters in a single task id:
	export const taskIdRegex = /[a-zA-Z0-9-_]+/;

taskIdRegex is then used in several other places in the code - so that if we ever change the rules, there is only one place to edit.

Allowed characters in IDs in Auto Suggest

When using the new feature in my own vault, I found that the new feature does not recognise - and _ as valid characters in IDs.

This means that with the following markdown, I cannot use auto-suggest to make 'another task' depend on a second task, as it does not recognise ⛔ 12_34 as a valid ID:

- [ ] hello world 🆔 12_34
- [ ] another task ⛔ 12_34, 

We could obviously just add those two characters to the regexes in the new Auto Suggest code, although my preference would be to eventually refactor the new code so that all ID regexes reuse taskIdRegex.

Understanding the regular expression I asked about

I previously put the new depends-on regular expression into regex101, with a selection of possible task lines in - sensible and not - to understand the behaviour of the regex: See https://regex101.com/r/0tPwyZ/1, and the screenshot below.

This is why I asked about the use of * as opposed to +.

I suspect that the regex may be too flexible, and the screenshot shows it is allowing users to construct 'depends-on' strings that Tasks would not actually parse (because it reads back from the end of the line until it finds something that is neither a recognised Tasks field nor a tag)

Adding tests

I've made some progress adding tests for the new code:

obsidian-tasks/tests/Suggestor/Suggestor.test.ts

Lines 251 to 325 in fd391f2

	describe('suggestions for dependency fields', () => {
	it('should offer "id" then "depends on" if user typed "id"', () => {
	const line = '- [ ] some task id';
	shouldStartWithSuggestionsEqualling(line, [`${idSymbol} Task ID`, `${dependsOnSymbol} Task depends on ID`]);
	});
	it('should offer to generate unique id if the id symbol is already present', () => {
	const line = `- [ ] some task ${idSymbol}`;
	shouldStartWithSuggestionsEqualling(line, ['Auto Generate Unique ID']);
	});
	it('should offer to depend on only task in vault, and include its filename in suggestion if user typed "id"', () => {
	const line = `- [ ] some task ${dependsOnSymbol} `;
	const taskToDependOn = TaskBuilder.createFullyPopulatedTask();
	shouldStartWithSuggestionsEqualling(line, ['Do exercises - From: fileName.md'], [taskToDependOn]);
	});
	// TODO should not offer to depend on self
	// TODO should offer tasks in current file before those in other files
	// TODO should not offer id or dependsOn if cursor is in the middle of the line
	// TODO test that it uses the same regex for Task IDs as the rest of the code
	// TODO confirm it does not unnecessarily rewrite tasks that already have an ID
	describe('suggesting additional dependencies', () => {
	const taskBuilder = new TaskBuilder().path('root/dir 1/dir 2/file-name.md');
	const allTasks = [
	// force line break
	taskBuilder.description('1').id('1234').build(),
	taskBuilder.description('2').id('5678').build(),
	];
	const suggestTask1 = '1 - From: file-name.md';
	const suggestTask2 = '2 - From: file-name.md';
	it('should suggest all tasks when there is no existing ID after dependsOn', () => {
	const line = `- [ ] some task ${dependsOnSymbol} `;
	shouldStartWithSuggestionsEqualling(line, [suggestTask1, suggestTask2], allTasks);
	});
	it('should offer tasks containing the search string, if given a partial ID', () => {
	// 1 does not match any of the existing IDs, so is presumed to be a substring to search for.
	const line = `- [ ] some task ${dependsOnSymbol} 1`;
	shouldStartWithSuggestionsEqualling(line, [suggestTask1], allTasks);
	});
	it('should only offer tasks not already depended upon - with 1 existing dependency', () => {
	const line = `- [ ] some task ${dependsOnSymbol} 1234,`;
	shouldStartWithSuggestionsEqualling(line, [suggestTask2], allTasks);
	});
	it.failing('should offer tasks when first existing dependency id has hyphen and underscore', () => {
	// TODO hyphen and underscore are not currently recognised in this location by the auto-suggest code
	const line = `- [ ] some task ${dependsOnSymbol} 1_2-3,`;
	shouldStartWithSuggestionsEqualling(line, [suggestTask1, suggestTask2], allTasks);
	});
	it('should only offer tasks not already depended upon - with all tasks already depended on', () => {
	const line = `- [ ] some task ${dependsOnSymbol} 1234,5678,`;
	const suggestions = buildSuggestionsForEndOfLine(line, allTasks);
	shouldOnlyOfferDefaultSuggestions(suggestions);
	});
	it('should not offer any tasks if there is not a comma after existing depends IDs', () => {
	const line = `- [ ] some task ${dependsOnSymbol} 1234,5678`;
	const suggestions = buildSuggestionsForEndOfLine(line, allTasks);
	shouldOnlyOfferDefaultSuggestions(suggestions);
	});
	it('should not offer tasks if "IDs" are separated by spaces', () => {
	const line = `- [ ] some task ${dependsOnSymbol} 1234 5678`;
	const suggestions = buildSuggestionsForEndOfLine(line, allTasks);
	shouldOnlyOfferDefaultSuggestions(suggestions);
	});
	});
	});

There is one test with .failing - which is the one that uses _ and - in an ID.

This would be the priority to fix, as it is definitely blocking release.

[claremacrae]

This is the full PR where I added some tests...
#2806

[claremacrae]

There is one test with .failing - which is the one that uses _ and - in an ID.

This would be the priority to fix, as it is definitely blocking release.

I'm working on this...

[claremacrae]

Hi @Genei180,

I just wanted to thank you again for this work, and give you a heads-up...

For reasons explained in PR #2828, and to enable me to release some other recent features, with a heavy heart I have had to temporarily turn off the features added in this PR.

The following isses can then be addressed without the pressure of needing to get a release out...

• Auto-suggest mechanism for dependencies seems to sometimes break an existing ID (likely when sync is enabled) #2823
• Adding dependsOn field via auto-suggest rewrites lines with existing IDs unnecessarily #2825
• Auto-suggest for dependencies restricts the characters that can be used when filtering task descriptions #2827

Thanks again. I look forward to being able to release this feature soon. 😄

[claremacrae]

I have just released Tasks 7.4.0, which supports Auto suggest for task dependencies. Thank you @Genei180.

For help and details, see Auto Suggest for Task dependencies.