Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
axelson wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
74fe90a
Toggle task done command adds global filter text
axelson Jun 3, 2023
86b4372
Add more tests
axelson Jun 4, 2023
6d116e6
Add regex testing
axelson Jun 4, 2023
750df8d
Add and respect autoInsertGlobalFilter setting
axelson Jun 4, 2023
05cca91
divide ToggleDone should add checkbox tests
axelson Jun 25, 2023
5ef7a10
Test when the global filter is at the end of the task
axelson Jun 25, 2023
31292fc
break up the should complete a task test
axelson Jun 25, 2023
ded74e8
Merge remote-tracking branch 'upstream/main' into insert-global-filter
axelson Jun 25, 2023
a523a0d
Clean up implementation
axelson Jun 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/Commands/ToggleDone.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { StatusRegistry } from '../StatusRegistry';

import { Task, TaskRegularExpressions } from '../Task';
import { TaskLocation } from '../TaskLocation';
import { GlobalFilter } from '../Config/GlobalFilter';

export const toggleDone = (checking: boolean, editor: Editor, view: MarkdownView | MarkdownFileInfo) => {
if (checking) {
Expand Down Expand Up @@ -91,7 +92,10 @@ export const toggleLine = (line: string, path: string): EditorInsertion => {
return { text: line.replace(TaskRegularExpressions.taskRegex, `$1- [${newStatusString}] $4`) };
} else if (TaskRegularExpressions.listItemRegex.test(line)) {
// Convert the list item to a checklist item.
const text = line.replace(TaskRegularExpressions.listItemRegex, '$1$2 [ ]');
const globalFilter = GlobalFilter.get();
const addGlobalFilter = globalFilter != '' && !GlobalFilter.includedIn(line);
const newTaskText = addGlobalFilter ? `[ ] ${globalFilter}` : '[ ]';
const text = line.replace(TaskRegularExpressions.listItemRegex, `$1$2 ${newTaskText}`);
Copy link
Copy Markdown
Member

@claremacrae claremacrae Jun 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It just dawned on me that someone recently spent a lot of time moving all code related to the global filter, including manipulating descriptions, in the new GlobalFilter class.

If the above cannot be done with the existing methods in GlobalFilter, I feel that the new code should go in to a new method in that class. The existing names there may give ideas.

Copy link
Copy Markdown
Contributor Author

@axelson axelson Jun 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah that makes sense. I took a stab at moving as much of the GlobalFilter-related logic to the GlobalFilter class as I could 750df8d. I didn't go all the way because I wanted to keep the regex-related logic in ToggleDone.ts. Let me know if this matches what you're thinking! If not then I'm open to suggestions.

return { text, moveTo: { ch: text.length } };
} else {
// Convert the line to a list item.
Expand Down
9 changes: 8 additions & 1 deletion src/TaskSerializer/DefaultTaskSerializer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,14 @@ export class DefaultTaskSerializer implements TaskSerializer {
// components but now we want them back.
// The goal is for a task of them form 'Do something #tag1 (due) tomorrow #tag2 (start) today'
// to actually have the description 'Do something #tag1 #tag2'
if (trailingTags.length > 0) line += ' ' + trailingTags;
if (trailingTags.length > 0) {
// If the line is empty besides the tag then don't prepend a space because it results in a double space
if (line === '') {
line += trailingTags;
} else {
line += ' ' + trailingTags;
}
}
Comment on lines +296 to +303
Copy link
Copy Markdown
Member

@claremacrae claremacrae Jun 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Various thoughts here...

What's the reason for this change in this PR? It seems unrelated.
What's the harm in a double-space? Presumably this was pre-existing behaviour?

Also, this is the code for the Tasks Emoji format.

So what happens when the user is using Dataview task format? Will it produce a double-space because it doesn't have this logic?

And what will happen if several more formats are added? Will they all need to have this check?


return {
description: line,
Expand Down
43 changes: 40 additions & 3 deletions tests/Commands/ToggleDone.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,20 +100,37 @@ describe('ToggleDone', () => {
});

it('should add checkbox to hyphen and space', () => {
GlobalFilter.set('');

testToggleLine('|- ', '- [ ] |');
testToggleLine('- |', '- [ ] |');
testToggleLine('- |foobar', '- [ ] foobar|');

GlobalFilter.set('#task');

testToggleLine('|- ', '- [ ] |');
testToggleLine('- |', '- [ ] |');
testToggleLine('- |foobar', '- [ ] foobar|');
testToggleLine('|- ', '- [ ] #task |');
testToggleLine('- |', '- [ ] #task |');
testToggleLine('- |foobar', '- [ ] #task foobar|');
testToggleLine('- |#task', '- [ ] #task|');

GlobalFilter.set('TODO');

testToggleLine('|- ', '- [ ] TODO |');
testToggleLine('- |', '- [ ] TODO |');
testToggleLine('- |foobar', '- [ ] TODO foobar|');
testToggleLine('- |TODO foobar', '- [ ] TODO foobar|');

// Test a global filter that has special characters from regular expressions
GlobalFilter.set('a.*b');

testToggleLine('|- [ ] a.*b ', '|- [x] a.*b ✅ 2022-09-04');
testToggleLine('- [ ] a.*b foobar |', '- [x] a.*b foobar |✅ 2022-09-04');
});

it('should complete a task', () => {
testToggleLine('|- [ ] ', '|- [x] ✅ 2022-09-04');
testToggleLine('- [ ] |', '- [x] | ✅ 2022-09-04');
testToggleLine('- [ ]| ', '- [x]| ✅ 2022-09-04');

// Issue #449 - cursor jumped 13 characters to the right on completion
testToggleLine('- [ ] I have a |proper description', '- [x] I have a |proper description ✅ 2022-09-04');
Expand All @@ -123,8 +140,28 @@ describe('ToggleDone', () => {
testToggleLine('|- [ ] ', '|- [x] ');
testToggleLine('- [ ] |', '- [x] |');

testToggleLine('|- [ ] #task ', '|- [x] #task ✅ 2022-09-04');
testToggleLine('- [ ] #task foobar |', '- [x] #task foobar |✅ 2022-09-04');

// Issue #449 - cursor jumped 13 characters to the right on completion
testToggleLine('- [ ] I have a |proper description', '- [x] I have a |proper description');

GlobalFilter.set('TODO');

testToggleLine('|- [ ] ', '|- [x] ');
testToggleLine('- [ ] |', '- [x] |');

testToggleLine('|- [ ] TODO ', '|- [x] TODO ✅ 2022-09-04');
testToggleLine('- [ ] TODO foobar |', '- [x] TODO foobar |✅ 2022-09-04');

// Test a global filter that has special characters from regular expressions
GlobalFilter.set('a.*b');

testToggleLine('|- [ ] ', '|- [x] ');
testToggleLine('- [ ] |', '- [x] |');

testToggleLine('|- [ ] a.*b ', '|- [x] a.*b ✅ 2022-09-04');
testToggleLine('- [ ] a.*b foobar |', '- [x] a.*b foobar |✅ 2022-09-04');
});

it('should un-complete a completed task', () => {
Expand Down
2 changes: 1 addition & 1 deletion tests/TaskSerializer/DataviewTaskSerializer.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ describe('DataviewTaskSerializer', () => {
});

it('should parse tags', () => {
const description = ' #hello #world #task';
const description = '#hello #world #task';
const taskDetails = deserialize(description);
expect(taskDetails).toMatchTaskDetails({ tags: ['#hello', '#world', '#task'], description });
});
Expand Down
2 changes: 1 addition & 1 deletion tests/TaskSerializer/DefaultTaskSerializer.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ describe.each(symbolMap)("DefaultTaskSerializer with '$taskFormat' symbols", ({
});

it('should parse tags', () => {
const description = ' #hello #world #task';
const description = '#hello #world #task';
const taskDetails = deserialize(description);
expect(taskDetails).toMatchTaskDetails({ tags: ['#hello', '#world', '#task'], description });
});
Expand Down