feat: Toggle task done command adds global filter text, if enabled

[axelson]

New behavior: if the user has a global filter set then the global filter is inserted when the "Tasks: Toggle task done" is run.

Description

The "Tasks: Toggle task done" command will now prepend the globalFilter when creating a new checkbox if the autoInsertGlobalFilter setting is enabled. The autoInsertGlobalFilter setting defaults to false.

Also fix an edge case where an extra empty space was added when the task doesn't have any description besides the global filter. Otherwise some of test cases felt broken because an extra space would be inserted.

Previously '- |#task' would become '- [ ] #task|' (notice the two spaces before #task) Now it becomes '- [ ] #task|'

TODO:

• [ ] Update this description to reflect all the changes (e.g. the new extractLineItemComponents)

Motivation and Context

Fixes #1320

I like to use the "Tasks: Toggle task done" command to create tasks, but since I have a global filter set I had to manually type the global filter each time. This PR changes the behavior so that if the user has a global filter set then the global filter is inserted when the "Tasks: Toggle task done" is run.

I'm not sure if this requires an update to the documentation. I didn't see anywhere where the behavior of "Tasks: Toggle task done" is explained. However this is a change in behavior that merits a notice in the release notes.

I'm not sure if this should be considered a breaking change.

Proposed release notes text

"Tasks: Toggle task done" now has the ability to prepend the global filter to tasks created with the "Tasks: Toggle task done" command if the "Tasks: Toggle task done" command inserts global task filter setting is enabled.

How has this been tested?

Tested in a test vault and updated the unit tests.

Screenshots (if appropriate)

Types of changes

Changes visible to users:

• [ ] Bug fix (prefix: fix - non-breaking change which fixes an issue)
• [x] 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)
• [x] 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

• [x] My code follows the code style of this project and passes yarn run lint.
• [ ] My change requires a change to the documentation.
• [x] I have updated the documentation accordingly.
• [x] My change has adequate Unit Test coverage.

Terms

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

[claremacrae]

Hi, thanks very much indeed for adding this.

I don't personally use a global filter, but I imagine that those who do will find it useful.

[claremacrae]

I'm not sure if this requires an update to the documentation. I didn't see anywhere where the behavior of "Tasks: Toggle task done" is explained.

Thanks for considering that. I searched the docs for 'command' and only found tangential references, so it's not something to include in the docs right now.

I've just made a note in #767 to ensure that the capability added here will (eventually) be documented...

However this is a change in behavior that merits a notice in the release notes.

Agreed. As a PR, it will automatically get included in the next release notes.

I'm not sure if this should be considered a breaking change.

Having re-read #1320, I am comfortable labelling this a bug fix, just because without this, the command adds lines which Tasks then ignores if the user has a global filter. That seems just wrong.

[ilandikov]

I'm a bit confused, in my workflow, I use Tasks to manage tasks with Global Filter and I also have tasks without it. I don't want Tasks to manage those. I use Toggle task cmd to complete both of them and this has been very useful - I have one cmd to complete both types of tasks.

So this kind of breaks it for me, there could be other people with this use-case. Maybe it could be possible to add this feature/bugfix with a setting? (Disabled by default)

[claremacrae]

I'm a bit confused, in my workflow, I use Tasks to manage tasks with Global Filter and I also have tasks without it. I don't want Tasks to manage those. I use Toggle task cmd to complete both of them and this has been very useful - I have one cmd to complete both types of tasks.

So this kind of breaks it for me, there could be other people with this use-case. Maybe it could be possible to add this feature/bugfix with a setting? (Disabled by default)

Thank you for saying this @ilandikov.

Overnight I had a nagging doubt about whether this might be the case, prompted by a dim recollection of someone else saying that they only used Tasks for a small subset of their checkboxes, and it was in their muscle memory to add the global filter in the few areas where they wanted Tasks to track.

So yes, I do think that having a setting to turn this PR on will be necessary to avoid disrupting people's muscle memory and workflow.

[axelson]

I'm a bit confused, in my workflow, I use Tasks to manage tasks with Global Filter and I also have tasks without it. I don't want Tasks to manage those. I use Toggle task cmd to complete both of them and this has been very useful - I have one cmd to complete both types of tasks.

So yes, I do think that having a setting to turn this PR on will be necessary to avoid disrupting people's muscle memory and workflow.

Ah I hadn't considered that use case because for me 90% percent of the checkboxes I create I want to be considered as Tasks plugin tasks. In 750df8d9070a0a60b1af589b90f60e49e111bdfb I've added a autoInsertGlobalFilter setting that controls this new behavior and defaults to false so there's no change for users unless they opt in to the new behavior.

[claremacrae]

Thank you again for offering this contribution, and apologies for not picking this up yet. It's not forgotten and I will get to it.

[axelson]

@claremacrae Thank you! And I'll try to fix the merge conflict over the weekend

[claremacrae]

@claremacrae Thank you! And I'll try to fix the merge conflict over the weekend

Thank you. That is useful to know - I will try and do the code review before the weekend then, so if there is any feedback you would only need to look at it once.

[axelson]

Okay I've pushed up some changes based on pairing as well as a few additional changes (such as extractLineItemComponents), I think the main items still remaining is to update the PR description to reflect all the changes and possibly a refactor that will allow us to reduce the repetition in the tests (i.e. a function tied directly to the auto insert behavior, possibly by testing addGlobalFilterToDescriptionDependingOnSettings directly instead of transitively testing it in ToggleDone).

[claremacrae]

Okay I've pushed up some changes based on pairing as well as a few additional changes (such as extractLineItemComponents),

Thanks!

I think the main items still remaining is to update the PR description to reflect all the changes

Am happy to see the PR description updated...

and possibly a refactor that will allow us to reduce the repetition in the tests (i.e. a function tied directly to the auto insert behavior, possibly by testing addGlobalFilterToDescriptionDependingOnSettings directly instead of transitively testing it in ToggleDone).

I have held off saying this as it feels a bit negative, but I basically feel that all the tests added in the PR are now in the wrong place.

The logic of the new behaviour should now be tested in the GlobalFilter tests.

I would pretty-much revert the changes to ToggleDone.test.ts and then add just these 2 tests:

• when autoAddGlobalFilter enabled, it only adds global filter at the point of making a list item in to a task
• when autoAddGlobalFilter disabled, it never adds the global filter

In other words, the tests in ToggleDone are written in terms of user-visible behaviour.

How do you feel about doing more in the testing?

If the description above is too vague, and if you are available on Sunday, we could pair on it together.

If that won't work, I don't mind editing the tests and pushing to your branch, and then merging.

[claremacrae]

Just to note that @ilandikov has added kindly merged in main and responded to the feedback requests here.

I will have time to review that over the next 2 or 3 weeks, and then figure out a way to first merge this one, and then to merge a PR from ilandikov to add the later stages - so that both contributors get credit...

Thanks to both @axelson and @ilandikov for working on this...

I hope that this message prevents the possible risk of two people working on it at the same time...

[ilandikov]

FYI: https://github.com/obsidian-tasks-group/obsidian-tasks/compare/main...ilandikov:obsidian-tasks:insert-global-filter

[claremacrae]

Setting to draft status any PRs which are not currently mergable...