Skip to content

Commit 428eff5

Browse files
claremacraeclaremacrae
authored
Merge pull request #3326 from ilandikov/refactor-task-location-in-list-item
refactor: add `TaskLocation` to `ListItem`
2 parents 28d0b34 + 3bf047f commit 428eff5

File tree

6 files changed

+61
-45
lines changed

6 files changed

+61
-45
lines changed

‎src/Obsidian/FileParser.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ export class FileParser {
119119
sectionIndex: number,
120120
) {
121121
if (listItem.task === undefined) {
122-
this.createListItem(listItem, line, lineNumber);
122+
this.createListItem(listItem, line, lineNumber, taskLocation);
123123
return sectionIndex;
124124
}
125125
let task;
@@ -149,16 +149,16 @@ export class FileParser {
149149
}
150150
} else {
151151
// Treat tasks without the global filter as list items
152-
this.createListItem(listItem, line, lineNumber);
152+
this.createListItem(listItem, line, lineNumber, taskLocation);
153153
}
154154
} catch (e) {
155155
this.errorReporter(e, this.filePath, listItem, line);
156156
}
157157
return sectionIndex;
158158
}
159159

160-
private createListItem(listItem: ListItemCache, line: string, lineNumber: number) {
160+
private createListItem(listItem: ListItemCache, line: string, lineNumber: number, taskLocation: TaskLocation) {
161161
const parentListItem: ListItem | null = this.line2ListItem.get(listItem.parent) ?? null;
162-
this.line2ListItem.set(lineNumber, new ListItem(line, parentListItem));
162+
this.line2ListItem.set(lineNumber, new ListItem(line, parentListItem, taskLocation));
163163
}
164164
}

‎src/Task/ListItem.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import type { TaskLocation } from './TaskLocation';
12
import { TaskRegularExpressions } from './TaskRegularExpressions';
23
import type { Task } from './Task';
34

@@ -10,7 +11,9 @@ export class ListItem {
1011
public readonly description: string;
1112
public readonly statusCharacter: string | null = null;
1213

13-
constructor(originalMarkdown: string, parent: ListItem | null) {
14+
public readonly taskLocation: TaskLocation;
15+
16+
constructor(originalMarkdown: string, parent: ListItem | null, taskLocation: TaskLocation) {
1417
this.description = originalMarkdown.replace(TaskRegularExpressions.listItemRegex, '').trim();
1518
const nonTaskMatch = RegExp(TaskRegularExpressions.nonTaskRegex).exec(originalMarkdown);
1619
if (nonTaskMatch) {
@@ -23,6 +26,8 @@ export class ListItem {
2326
if (parent !== null) {
2427
parent.children.push(this);
2528
}
29+
30+
this.taskLocation = taskLocation;
2631
}
2732

2833
/**

‎src/Task/Task.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ export class Task extends ListItem {
120120
scheduledDateIsInferred: boolean;
121121
parent?: ListItem | null;
122122
}) {
123-
super(originalMarkdown, parent);
123+
super(originalMarkdown, parent, taskLocation);
124124
// NEW_TASK_FIELD_EDIT_REQUIRED
125125
this.status = status;
126126
this.description = description;

‎tests/Obsidian/Cache.test.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,12 @@ describe('cache', () => {
513513
- list item child : ListItem
514514
"
515515
`);
516+
517+
const task = tasks[0];
518+
expect(task.taskLocation.lineNumber).toEqual(0);
519+
expect(task.children[0].taskLocation.lineNumber).toEqual(1);
520+
expect(task.children[1].taskLocation.lineNumber).toEqual(2);
521+
expect(task.children[2].taskLocation.lineNumber).toEqual(3);
516522
});
517523

518524
it('callout', () => {

‎tests/Task/ListItem.test.ts‎

Lines changed: 41 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -12,34 +12,37 @@ import { createChildListItem } from './ListItemHelpers';
1212

1313
window.moment = moment;
1414

15+
const taskLocation = TaskLocation.fromUnknownPosition(new TasksFile('anything.md'));
16+
1517
describe('list item tests', () => {
1618
it('should create list item with empty children and absent parent', () => {
17-
const listItem = new ListItem('', null);
19+
const listItem = new ListItem('', null, taskLocation);
1820
expect(listItem).toBeDefined();
1921
expect(listItem.children).toEqual([]);
2022
expect(listItem.parent).toEqual(null);
23+
expect(listItem.taskLocation).toBe(taskLocation);
2124
});
2225

2326
it('should create a list item with 2 children', () => {
24-
const listItem = new ListItem('', null);
25-
const childItem1 = new ListItem('', listItem);
26-
const childItem2 = new ListItem('', listItem);
27+
const listItem = new ListItem('', null, taskLocation);
28+
const childItem1 = new ListItem('', listItem, taskLocation);
29+
const childItem2 = new ListItem('', listItem, taskLocation);
2730
expect(listItem).toBeDefined();
2831
expect(childItem1.parent).toEqual(listItem);
2932
expect(childItem2.parent).toEqual(listItem);
3033
expect(listItem.children).toEqual([childItem1, childItem2]);
3134
});
3235

3336
it('should create a list item with a parent', () => {
34-
const parentItem = new ListItem('', null);
35-
const listItem = new ListItem('', parentItem);
37+
const parentItem = new ListItem('', null, taskLocation);
38+
const listItem = new ListItem('', parentItem, taskLocation);
3639
expect(listItem).toBeDefined();
3740
expect(listItem.parent).toEqual(parentItem);
3841
expect(parentItem.children).toEqual([listItem]);
3942
});
4043

4144
it('should create a task child for a list item parent', () => {
42-
const parentListItem = new ListItem('- parent item', null);
45+
const parentListItem = new ListItem('- parent item', null, taskLocation);
4346
const firstReadTask = Task.fromLine({
4447
line: ' - [ ] child task',
4548
taskLocation: TaskLocation.fromUnknownPosition(new TasksFile('x.md')),
@@ -59,16 +62,16 @@ describe('list item tests', () => {
5962
taskLocation: TaskLocation.fromUnknownPosition(new TasksFile('x.md')),
6063
fallbackDate: null,
6164
});
62-
const childListItem = new ListItem(' - child item', parentTask);
65+
const childListItem = new ListItem(' - child item', parentTask, taskLocation);
6366

6467
expect(parentTask!.children).toEqual([childListItem]);
6568
expect(childListItem.parent).toBe(parentTask);
6669
});
6770

6871
it('should identify root of the hierarchy', () => {
69-
const grandParent = new ListItem('- grand parent', null);
70-
const parent = new ListItem('- parent', grandParent);
71-
const child = new ListItem('- child', parent);
72+
const grandParent = new ListItem('- grand parent', null, taskLocation);
73+
const parent = new ListItem('- parent', grandParent, taskLocation);
74+
const child = new ListItem('- child', parent, taskLocation);
7275

7376
expect(grandParent.root.originalMarkdown).toEqual('- grand parent');
7477
expect(parent.root.originalMarkdown).toEqual('- grand parent');
@@ -80,7 +83,7 @@ describe('list item tests', () => {
8083
});
8184

8285
it('should not be a task', () => {
83-
const listItem = new ListItem('- list item', null);
86+
const listItem = new ListItem('- list item', null, taskLocation);
8487
expect(listItem.isTask).toBe(false);
8588
});
8689

@@ -95,7 +98,7 @@ describe('list item tests', () => {
9598
])('should parse description with list item prefix: "%s"', (prefix: string, shouldPass) => {
9699
const description = 'stuff';
97100
const line = prefix + description;
98-
const listItem = new ListItem(line, null);
101+
const listItem = new ListItem(line, null, taskLocation);
99102
expect(listItem.originalMarkdown).toEqual(line);
100103
if (shouldPass) {
101104
expect(listItem.description).toEqual(description);
@@ -107,23 +110,23 @@ describe('list item tests', () => {
107110

108111
describe('list item parsing', () => {
109112
it('should read a list item without checkbox', () => {
110-
const item = new ListItem('- without checkbox', null);
113+
const item = new ListItem('- without checkbox', null, taskLocation);
111114

112115
expect(item.description).toEqual('without checkbox');
113116
expect(item.originalMarkdown).toEqual('- without checkbox');
114117
expect(item.statusCharacter).toEqual(null);
115118
});
116119

117120
it('should read a list item with checkbox', () => {
118-
const item = new ListItem('- [ ] with checkbox', null);
121+
const item = new ListItem('- [ ] with checkbox', null, taskLocation);
119122

120123
expect(item.description).toEqual('with checkbox');
121124
expect(item.originalMarkdown).toEqual('- [ ] with checkbox');
122125
expect(item.statusCharacter).toEqual(' ');
123126
});
124127

125128
it('should read a list item with checkbox', () => {
126-
const item = new ListItem('- [x] with checked checkbox', null);
129+
const item = new ListItem('- [x] with checked checkbox', null, taskLocation);
127130

128131
expect(item.description).toEqual('with checked checkbox');
129132
expect(item.originalMarkdown).toEqual('- [x] with checked checkbox');
@@ -133,7 +136,7 @@ describe('list item parsing', () => {
133136
it('should accept a non list item', () => {
134137
// we tried making the constructor throw if given a non list item
135138
// but it broke lots of normal Task uses in the tests (TaskBuilder)
136-
const item = new ListItem('# Heading', null);
139+
const item = new ListItem('# Heading', null, taskLocation);
137140

138141
expect(item.description).toEqual('# Heading');
139142
expect(item.originalMarkdown).toEqual('# Heading');
@@ -144,8 +147,8 @@ describe('list item parsing', () => {
144147
describe('related items', () => {
145148
it('should detect if no closest parent task', () => {
146149
const task = fromLine({ line: '- [ ] task' });
147-
const item = new ListItem('- item', null);
148-
const childOfItem = new ListItem('- child of item', item);
150+
const item = new ListItem('- item', null, taskLocation);
151+
const childOfItem = new ListItem('- child of item', item, taskLocation);
149152

150153
expect(task.findClosestParentTask()).toEqual(null);
151154
expect(item.findClosestParentTask()).toEqual(null);
@@ -154,8 +157,8 @@ describe('related items', () => {
154157

155158
it('should find the closest parent task', () => {
156159
const parentTask = fromLine({ line: '- [ ] task' });
157-
const child = new ListItem('- item', parentTask);
158-
const grandChild = new ListItem('- item', child);
160+
const child = new ListItem('- item', parentTask, taskLocation);
161+
const grandChild = new ListItem('- item', child, taskLocation);
159162

160163
expect(parentTask.findClosestParentTask()).toEqual(null);
161164
expect(child.findClosestParentTask()).toEqual(parentTask);
@@ -165,45 +168,45 @@ describe('related items', () => {
165168

166169
describe('identicalTo', () => {
167170
it('should test same markdown', () => {
168-
const listItem1 = new ListItem('- same description', null);
169-
const listItem2 = new ListItem('- same description', null);
171+
const listItem1 = new ListItem('- same description', null, taskLocation);
172+
const listItem2 = new ListItem('- same description', null, taskLocation);
170173
expect(listItem1.identicalTo(listItem2)).toEqual(true);
171174
});
172175

173176
it('should test different markdown', () => {
174-
const listItem1 = new ListItem('- description', null);
175-
const listItem2 = new ListItem('- description two', null);
177+
const listItem1 = new ListItem('- description', null, taskLocation);
178+
const listItem2 = new ListItem('- description two', null, taskLocation);
176179
expect(listItem1.identicalTo(listItem2)).toEqual(false);
177180
});
178181

179182
it('should recognise list items with different number of children', () => {
180-
const item1 = new ListItem('- item', null);
183+
const item1 = new ListItem('- item', null, taskLocation);
181184
createChildListItem('- child of item1', item1);
182185

183-
const item2 = new ListItem('- item', null);
186+
const item2 = new ListItem('- item', null, taskLocation);
184187

185188
expect(item2.identicalTo(item1)).toEqual(false);
186189
});
187190

188191
it('should recognise list items with different children', () => {
189-
const item1 = new ListItem('- item', null);
192+
const item1 = new ListItem('- item', null, taskLocation);
190193
createChildListItem('- child of item1', item1);
191194

192-
const item2 = new ListItem('- item', null);
195+
const item2 = new ListItem('- item', null, taskLocation);
193196
createChildListItem('- child of item2', item2);
194197

195198
expect(item2.identicalTo(item1)).toEqual(false);
196199
});
197200

198201
it('should recognise different status characters', () => {
199-
const item1 = new ListItem('- [1] item', null);
200-
const item2 = new ListItem('- [2] item', null);
202+
const item1 = new ListItem('- [1] item', null, taskLocation);
203+
const item2 = new ListItem('- [2] item', null, taskLocation);
201204

202205
expect(item2.identicalTo(item1)).toEqual(false);
203206
});
204207

205208
it('should recognise ListItem and Task as different', () => {
206-
const listItem = new ListItem('- [ ] description', null);
209+
const listItem = new ListItem('- [ ] description', null, taskLocation);
207210
const task = fromLine({ line: '- [ ] description' });
208211

209212
expect(listItem.identicalTo(task)).toEqual(false);
@@ -219,19 +222,19 @@ describe('checking if list item lists are identical', () => {
219222

220223
it('should treat different sized lists as different', () => {
221224
const list1: ListItem[] = [];
222-
const list2: ListItem[] = [new ListItem('- x', null)];
225+
const list2: ListItem[] = [new ListItem('- x', null, taskLocation)];
223226
expect(ListItem.listsAreIdentical(list1, list2)).toBe(false);
224227
});
225228

226229
it('should detect matching list items as same', () => {
227-
const list1: ListItem[] = [new ListItem('- 1', null)];
228-
const list2: ListItem[] = [new ListItem('- 1', null)];
230+
const list1: ListItem[] = [new ListItem('- 1', null, taskLocation)];
231+
const list2: ListItem[] = [new ListItem('- 1', null, taskLocation)];
229232
expect(ListItem.listsAreIdentical(list1, list2)).toBe(true);
230233
});
231234

232235
it('- should detect non-matching list items as different', () => {
233-
const list1: ListItem[] = [new ListItem('- 1', null)];
234-
const list2: ListItem[] = [new ListItem('- 2', null)];
236+
const list1: ListItem[] = [new ListItem('- 1', null, taskLocation)];
237+
const list2: ListItem[] = [new ListItem('- 2', null, taskLocation)];
235238
expect(ListItem.listsAreIdentical(list1, list2)).toBe(false);
236239
});
237240
});
@@ -264,7 +267,7 @@ describe('checking if task lists are identical', () => {
264267

265268
describe('checking if mixed lists are identical', () => {
266269
it('should recognise mixed lists as unequal', () => {
267-
const list1 = [new ListItem('- [ ] description', null)];
270+
const list1 = [new ListItem('- [ ] description', null, taskLocation)];
268271
const list2 = [fromLine({ line: '- [ ] description' })];
269272

270273
expect(ListItem.listsAreIdentical(list1, list1)).toEqual(true);

‎tests/Task/ListItemHelpers.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import { ListItem } from '../../src/Task/ListItem';
2+
import { TaskLocation } from '../../src/Task/TaskLocation';
23

34
export function createChildListItem(originalMarkdown: string, parent: ListItem) {
45
// This exists purely to silence WebStorm about typescript:S1848
56
// See https://sonarcloud.io/organizations/obsidian-tasks-group/rules?open=typescript%3AS1848&rule_key=typescript%3AS1848
6-
new ListItem(originalMarkdown, parent);
7+
const taskLocation = TaskLocation.fromUnknownPosition(parent.taskLocation.tasksFile);
8+
new ListItem(originalMarkdown, parent, taskLocation);
79
}

0 commit comments

Comments
 (0)