From d7fc0dce3f46fa2cdf418bab63e1e6acb6b9dc9f Mon Sep 17 00:00:00 2001 From: Ilyas Landikov Date: Sat, 20 May 2023 16:00:02 +0600 Subject: [PATCH 01/21] test: document current group sorting of PriorityField --- tests/Query/Filter/PriorityField.test.ts | 47 ++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/Query/Filter/PriorityField.test.ts b/tests/Query/Filter/PriorityField.test.ts index d909fb3991..f7b833fb3c 100644 --- a/tests/Query/Filter/PriorityField.test.ts +++ b/tests/Query/Filter/PriorityField.test.ts @@ -9,6 +9,7 @@ import { expectTaskComparesBefore, expectTaskComparesEqual, } from '../../CustomMatchers/CustomMatchersForSorting'; +import { TaskGroups } from '../../../src/Query/TaskGroups'; function testTaskFilterForTaskWithPriority(filter: string, priority: Priority, expected: boolean) { const builder = new TaskBuilder(); @@ -174,4 +175,50 @@ describe('grouping by priority', () => { // Assert expect(grouper(fromLine({ line: taskLine }))).toEqual(groups); }); + + it('task "%s" should have groups: %s', () => { + // Arrange + const tasks = [ + fromLine({ line: '- [ ] a 🔽' }), + fromLine({ line: '- [ ] a ⏫' }), + fromLine({ line: '- [ ] a' }), + fromLine({ line: '- [ ] a 🔼' }), + ]; + + const grouper = [new PriorityField().createNormalGrouper()]; + const groups = new TaskGroups(grouper, tasks); + + // Assert + expect(groups.toString()).toMatchInlineSnapshot(` + "Groupers (if any): + - priority + + Group names: [Priority 1: High] + #### [priority] Priority 1: High + - [ ] a ⏫ + + --- + + Group names: [Priority 2: Medium] + #### [priority] Priority 2: Medium + - [ ] a 🔼 + + --- + + Group names: [Priority 3: None] + #### [priority] Priority 3: None + - [ ] a + + --- + + Group names: [Priority 4: Low] + #### [priority] Priority 4: Low + - [ ] a 🔽 + + --- + + 4 tasks + " + `); + }); }); From 1acce881da502c3b92bf7a6119b54cca629d4a5f Mon Sep 17 00:00:00 2001 From: Ilyas Landikov Date: Sat, 20 May 2023 16:27:08 +0600 Subject: [PATCH 02/21] refactor: move localeCompare to groupSorter --- src/Query/Grouper.ts | 17 ++++++++++++++++- src/Query/TaskGroups.ts | 2 +- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/Query/Grouper.ts b/src/Query/Grouper.ts index 8df4050e66..647e6248f4 100644 --- a/src/Query/Grouper.ts +++ b/src/Query/Grouper.ts @@ -10,6 +10,13 @@ import type { Task } from '../Task'; */ export type GrouperFunction = (task: Task) => string[]; +type GroupSorter = (groupName1: string, groupName2: string) => number; +export default GroupSorter; + +export const defaultGroupSorter = (groupName1: string, groupName2: string) => { + return groupName1.localeCompare(groupName2, undefined, { numeric: true }); +}; + /** * A named function that is used to determine the group heading name(s) to use for a {@link Task} object. * @@ -36,9 +43,17 @@ export class Grouper { public readonly reverse: boolean; - constructor(property: string, grouper: GrouperFunction, reverse: boolean) { + public readonly groupSorter: GroupSorter; + + constructor( + property: string, + grouper: GrouperFunction, + reverse: boolean, + _groupSorter: GroupSorter = defaultGroupSorter, + ) { this.property = property; this.grouper = grouper; this.reverse = reverse; + this.groupSorter = _groupSorter; } } diff --git a/src/Query/TaskGroups.ts b/src/Query/TaskGroups.ts index 92ba9f9af8..35a28d2c6b 100644 --- a/src/Query/TaskGroups.ts +++ b/src/Query/TaskGroups.ts @@ -116,7 +116,7 @@ export class TaskGroups { // which will likely involve adjusting this code to sort by applying a Comparator // to the first Task in each group. const grouper = this._groupers[i]; - const result = groupNames1[i].localeCompare(groupNames2[i], undefined, { numeric: true }); + const result = grouper.groupSorter(groupNames1[i], groupNames2[i]); if (result !== 0) { return grouper.reverse ? -result : result; } From 1e7e92c0956f0fe315261fc50a783f53f09aa694 Mon Sep 17 00:00:00 2001 From: Ilyas Landikov Date: Sat, 20 May 2023 17:56:47 +0600 Subject: [PATCH 03/21] refactor: move groupSorter to Field --- src/Query/Filter/Field.ts | 10 ++++++++-- src/Query/Filter/MultiTextField.ts | 2 +- src/Query/Grouper.ts | 16 +++------------- 3 files changed, 12 insertions(+), 16 deletions(-) diff --git a/src/Query/Filter/Field.ts b/src/Query/Filter/Field.ts index b342583189..8c1950a455 100644 --- a/src/Query/Filter/Field.ts +++ b/src/Query/Filter/Field.ts @@ -1,7 +1,7 @@ import { Sorter } from '../Sorter'; import type { Comparator } from '../Sorter'; import * as RegExpTools from '../../lib/RegExpTools'; -import { Grouper } from '../Grouper'; +import { type GroupSorter, Grouper } from '../Grouper'; import type { GrouperFunction } from '../Grouper'; import type { FilterOrErrorMessage } from './Filter'; @@ -355,12 +355,18 @@ export abstract class Field { throw Error(`grouper() unimplemented for ${this.fieldNameSingular()}`); } + protected groupSorter(): GroupSorter { + return (groupName1: string, groupName2: string) => { + return groupName1.localeCompare(groupName2, undefined, { numeric: true }); + }; + } + /** * Create a {@link Grouper} object for grouping tasks by this field's value. * @param reverse - false for normal group order, true for reverse group order. */ public createGrouper(reverse: boolean): Grouper { - return new Grouper(this.fieldNameSingular(), this.grouper(), reverse); + return new Grouper(this.fieldNameSingular(), this.grouper(), reverse, this.groupSorter()); } /** diff --git a/src/Query/Filter/MultiTextField.ts b/src/Query/Filter/MultiTextField.ts index ef6a0d70df..b87c881519 100644 --- a/src/Query/Filter/MultiTextField.ts +++ b/src/Query/Filter/MultiTextField.ts @@ -64,7 +64,7 @@ export abstract class MultiTextField extends TextField { * This overloads {@link Field.createGrouper} to put a plural field name in the {@link Grouper.property}. */ public createGrouper(reverse: boolean): Grouper { - return new Grouper(this.fieldNamePlural(), this.grouper(), reverse); + return new Grouper(this.fieldNamePlural(), this.grouper(), reverse, this.groupSorter()); } protected grouperRegExp(): RegExp { diff --git a/src/Query/Grouper.ts b/src/Query/Grouper.ts index 647e6248f4..eb463b8538 100644 --- a/src/Query/Grouper.ts +++ b/src/Query/Grouper.ts @@ -10,12 +10,7 @@ import type { Task } from '../Task'; */ export type GrouperFunction = (task: Task) => string[]; -type GroupSorter = (groupName1: string, groupName2: string) => number; -export default GroupSorter; - -export const defaultGroupSorter = (groupName1: string, groupName2: string) => { - return groupName1.localeCompare(groupName2, undefined, { numeric: true }); -}; +export type GroupSorter = (groupName1: string, groupName2: string) => number; /** * A named function that is used to determine the group heading name(s) to use for a {@link Task} object. @@ -45,15 +40,10 @@ export class Grouper { public readonly groupSorter: GroupSorter; - constructor( - property: string, - grouper: GrouperFunction, - reverse: boolean, - _groupSorter: GroupSorter = defaultGroupSorter, - ) { + constructor(property: string, grouper: GrouperFunction, reverse: boolean, groupSorter: GroupSorter) { this.property = property; this.grouper = grouper; this.reverse = reverse; - this.groupSorter = _groupSorter; + this.groupSorter = groupSorter; } } From edca28fa687db6276bb359af4ee4b0a8149d98f0 Mon Sep 17 00:00:00 2001 From: Ilyas Landikov Date: Sat, 20 May 2023 18:00:13 +0600 Subject: [PATCH 04/21] refactor: rename sorter to comparator --- src/Query/Filter/Field.ts | 4 ++-- src/Query/Filter/MultiTextField.ts | 2 +- src/Query/Grouper.ts | 6 +++--- src/Query/TaskGroups.ts | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Query/Filter/Field.ts b/src/Query/Filter/Field.ts index 8c1950a455..dbb6031a6d 100644 --- a/src/Query/Filter/Field.ts +++ b/src/Query/Filter/Field.ts @@ -355,7 +355,7 @@ export abstract class Field { throw Error(`grouper() unimplemented for ${this.fieldNameSingular()}`); } - protected groupSorter(): GroupSorter { + protected groupComparator(): GroupSorter { return (groupName1: string, groupName2: string) => { return groupName1.localeCompare(groupName2, undefined, { numeric: true }); }; @@ -366,7 +366,7 @@ export abstract class Field { * @param reverse - false for normal group order, true for reverse group order. */ public createGrouper(reverse: boolean): Grouper { - return new Grouper(this.fieldNameSingular(), this.grouper(), reverse, this.groupSorter()); + return new Grouper(this.fieldNameSingular(), this.grouper(), reverse, this.groupComparator()); } /** diff --git a/src/Query/Filter/MultiTextField.ts b/src/Query/Filter/MultiTextField.ts index b87c881519..7f95255863 100644 --- a/src/Query/Filter/MultiTextField.ts +++ b/src/Query/Filter/MultiTextField.ts @@ -64,7 +64,7 @@ export abstract class MultiTextField extends TextField { * This overloads {@link Field.createGrouper} to put a plural field name in the {@link Grouper.property}. */ public createGrouper(reverse: boolean): Grouper { - return new Grouper(this.fieldNamePlural(), this.grouper(), reverse, this.groupSorter()); + return new Grouper(this.fieldNamePlural(), this.grouper(), reverse, this.groupComparator()); } protected grouperRegExp(): RegExp { diff --git a/src/Query/Grouper.ts b/src/Query/Grouper.ts index eb463b8538..75e6b542e2 100644 --- a/src/Query/Grouper.ts +++ b/src/Query/Grouper.ts @@ -38,12 +38,12 @@ export class Grouper { public readonly reverse: boolean; - public readonly groupSorter: GroupSorter; + public readonly groupComparator: GroupSorter; - constructor(property: string, grouper: GrouperFunction, reverse: boolean, groupSorter: GroupSorter) { + constructor(property: string, grouper: GrouperFunction, reverse: boolean, groupComparator: GroupSorter) { this.property = property; this.grouper = grouper; this.reverse = reverse; - this.groupSorter = groupSorter; + this.groupComparator = groupComparator; } } diff --git a/src/Query/TaskGroups.ts b/src/Query/TaskGroups.ts index 35a28d2c6b..5ee457ede4 100644 --- a/src/Query/TaskGroups.ts +++ b/src/Query/TaskGroups.ts @@ -116,7 +116,7 @@ export class TaskGroups { // which will likely involve adjusting this code to sort by applying a Comparator // to the first Task in each group. const grouper = this._groupers[i]; - const result = grouper.groupSorter(groupNames1[i], groupNames2[i]); + const result = grouper.groupComparator(groupNames1[i], groupNames2[i]); if (result !== 0) { return grouper.reverse ? -result : result; } From 3f1cb45299c1677481b56b9f57865c023a485b01 Mon Sep 17 00:00:00 2001 From: Ilyas Landikov Date: Sat, 20 May 2023 18:02:07 +0600 Subject: [PATCH 05/21] refactor: move groupComparator() to the bottom --- src/Query/Filter/Field.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Query/Filter/Field.ts b/src/Query/Filter/Field.ts index dbb6031a6d..623a7c5a11 100644 --- a/src/Query/Filter/Field.ts +++ b/src/Query/Filter/Field.ts @@ -355,12 +355,6 @@ export abstract class Field { throw Error(`grouper() unimplemented for ${this.fieldNameSingular()}`); } - protected groupComparator(): GroupSorter { - return (groupName1: string, groupName2: string) => { - return groupName1.localeCompare(groupName2, undefined, { numeric: true }); - }; - } - /** * Create a {@link Grouper} object for grouping tasks by this field's value. * @param reverse - false for normal group order, true for reverse group order. @@ -388,4 +382,10 @@ export abstract class Field { public createReverseGrouper(): Grouper { return this.createGrouper(true); } + + protected groupComparator(): GroupSorter { + return (groupName1: string, groupName2: string) => { + return groupName1.localeCompare(groupName2, undefined, { numeric: true }); + }; + } } From 5e827e4644798c9b0bc10119bca68d54b7ae3680 Mon Sep 17 00:00:00 2001 From: Ilyas Landikov Date: Sat, 20 May 2023 18:15:20 +0600 Subject: [PATCH 06/21] refactor: custom groupComparator() in PriorityField --- src/Query/Filter/PriorityField.ts | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/Query/Filter/PriorityField.ts b/src/Query/Filter/PriorityField.ts index bf14ea596c..9ed2358abc 100644 --- a/src/Query/Filter/PriorityField.ts +++ b/src/Query/Filter/PriorityField.ts @@ -1,7 +1,7 @@ import { Priority, Task } from '../../Task'; import { Explanation } from '../Explain/Explanation'; import type { Comparator } from '../Sorter'; -import type { GrouperFunction } from '../Grouper'; +import type { GroupSorter, GrouperFunction } from '../Grouper'; import { Field } from './Field'; import { Filter, FilterOrErrorMessage } from './Filter'; @@ -105,4 +105,19 @@ export class PriorityField extends Field { return [`Priority ${task.priority}: ${priorityName}`]; }; } + + protected groupComparator(): GroupSorter { + return (groupName1: string, groupName2: string) => { + const comparatorArray = [ + `Priority ${Priority.High}: High`, + `Priority ${Priority.Medium}: Medium`, + `Priority ${Priority.None}: None`, + `Priority ${Priority.Low}: Low`, + ]; + + const groupName1Index = comparatorArray.indexOf(groupName1); + const groupName2Index = comparatorArray.indexOf(groupName2); + return groupName1Index - groupName2Index; + }; + } } From e9355f22571e02b7e350f133a7fa90227e4fbd1b Mon Sep 17 00:00:00 2001 From: Ilyas Landikov Date: Sat, 20 May 2023 18:59:21 +0600 Subject: [PATCH 07/21] refactor: add Priority.toNumber() --- src/Task.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/Task.ts b/src/Task.ts index 741fb5d5ff..fdceb2c319 100644 --- a/src/Task.ts +++ b/src/Task.ts @@ -25,6 +25,22 @@ export enum Priority { Low = '4', } +export namespace Priority { + export function toNumber(priority: string): number { + switch (priority) { + case 'High': + return 1; + case 'Medium': + return 2; + case 'None': + return 3; + case 'Low': + return 4; + } + return 1; + } +} + export class TaskRegularExpressions { public static readonly dateFormat = 'YYYY-MM-DD'; From d0a078f597d9e9cf72864d7894203da4e44a5c0d Mon Sep 17 00:00:00 2001 From: Ilyas Landikov Date: Sat, 20 May 2023 19:05:26 +0600 Subject: [PATCH 08/21] feat: make Priority group names shorter --- src/Query/Filter/PriorityField.ts | 13 ++----------- tests/Query/Filter/PriorityField.test.ts | 24 ++++++++++++------------ 2 files changed, 14 insertions(+), 23 deletions(-) diff --git a/src/Query/Filter/PriorityField.ts b/src/Query/Filter/PriorityField.ts index 9ed2358abc..0390df6401 100644 --- a/src/Query/Filter/PriorityField.ts +++ b/src/Query/Filter/PriorityField.ts @@ -102,22 +102,13 @@ export class PriorityField extends Field { priorityName = 'Low'; break; } - return [`Priority ${task.priority}: ${priorityName}`]; + return [`${priorityName}`]; }; } protected groupComparator(): GroupSorter { return (groupName1: string, groupName2: string) => { - const comparatorArray = [ - `Priority ${Priority.High}: High`, - `Priority ${Priority.Medium}: Medium`, - `Priority ${Priority.None}: None`, - `Priority ${Priority.Low}: Low`, - ]; - - const groupName1Index = comparatorArray.indexOf(groupName1); - const groupName2Index = comparatorArray.indexOf(groupName2); - return groupName1Index - groupName2Index; + return Priority.toNumber(groupName1) - Priority.toNumber(groupName2); }; } } diff --git a/tests/Query/Filter/PriorityField.test.ts b/tests/Query/Filter/PriorityField.test.ts index f7b833fb3c..80e867eeb6 100644 --- a/tests/Query/Filter/PriorityField.test.ts +++ b/tests/Query/Filter/PriorityField.test.ts @@ -164,10 +164,10 @@ describe('grouping by priority', () => { }); it.each([ - ['- [ ] a ⏫', ['Priority 1: High']], - ['- [ ] a 🔼', ['Priority 2: Medium']], - ['- [ ] a', ['Priority 3: None']], - ['- [ ] a 🔽', ['Priority 4: Low']], + ['- [ ] a ⏫', ['High']], + ['- [ ] a 🔼', ['Medium']], + ['- [ ] a', ['None']], + ['- [ ] a 🔽', ['Low']], ])('task "%s" should have groups: %s', (taskLine: string, groups: string[]) => { // Arrange const grouper = new PriorityField().createNormalGrouper().grouper; @@ -193,26 +193,26 @@ describe('grouping by priority', () => { "Groupers (if any): - priority - Group names: [Priority 1: High] - #### [priority] Priority 1: High + Group names: [High] + #### [priority] High - [ ] a ⏫ --- - Group names: [Priority 2: Medium] - #### [priority] Priority 2: Medium + Group names: [Medium] + #### [priority] Medium - [ ] a 🔼 --- - Group names: [Priority 3: None] - #### [priority] Priority 3: None + Group names: [None] + #### [priority] None - [ ] a --- - Group names: [Priority 4: Low] - #### [priority] Priority 4: Low + Group names: [Low] + #### [priority] Low - [ ] a 🔽 --- From 090c195fefe271e66b0490ca6ed809c9a05cb951 Mon Sep 17 00:00:00 2001 From: Ilyas Landikov Date: Sat, 20 May 2023 19:10:56 +0600 Subject: [PATCH 09/21] refactor: rename types --- src/Query/Filter/Field.ts | 4 ++-- src/Query/Filter/PriorityField.ts | 4 ++-- src/Query/Grouper.ts | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Query/Filter/Field.ts b/src/Query/Filter/Field.ts index 623a7c5a11..3f109c2b6e 100644 --- a/src/Query/Filter/Field.ts +++ b/src/Query/Filter/Field.ts @@ -1,7 +1,7 @@ import { Sorter } from '../Sorter'; import type { Comparator } from '../Sorter'; import * as RegExpTools from '../../lib/RegExpTools'; -import { type GroupSorter, Grouper } from '../Grouper'; +import { type GroupComparator, Grouper } from '../Grouper'; import type { GrouperFunction } from '../Grouper'; import type { FilterOrErrorMessage } from './Filter'; @@ -383,7 +383,7 @@ export abstract class Field { return this.createGrouper(true); } - protected groupComparator(): GroupSorter { + protected groupComparator(): GroupComparator { return (groupName1: string, groupName2: string) => { return groupName1.localeCompare(groupName2, undefined, { numeric: true }); }; diff --git a/src/Query/Filter/PriorityField.ts b/src/Query/Filter/PriorityField.ts index 0390df6401..33631d7d29 100644 --- a/src/Query/Filter/PriorityField.ts +++ b/src/Query/Filter/PriorityField.ts @@ -1,7 +1,7 @@ import { Priority, Task } from '../../Task'; import { Explanation } from '../Explain/Explanation'; import type { Comparator } from '../Sorter'; -import type { GroupSorter, GrouperFunction } from '../Grouper'; +import type { GroupComparator as GroupComparator, GrouperFunction } from '../Grouper'; import { Field } from './Field'; import { Filter, FilterOrErrorMessage } from './Filter'; @@ -106,7 +106,7 @@ export class PriorityField extends Field { }; } - protected groupComparator(): GroupSorter { + protected groupComparator(): GroupComparator { return (groupName1: string, groupName2: string) => { return Priority.toNumber(groupName1) - Priority.toNumber(groupName2); }; diff --git a/src/Query/Grouper.ts b/src/Query/Grouper.ts index 75e6b542e2..36d6af343d 100644 --- a/src/Query/Grouper.ts +++ b/src/Query/Grouper.ts @@ -10,7 +10,7 @@ import type { Task } from '../Task'; */ export type GrouperFunction = (task: Task) => string[]; -export type GroupSorter = (groupName1: string, groupName2: string) => number; +export type GroupComparator = (groupName1: string, groupName2: string) => number; /** * A named function that is used to determine the group heading name(s) to use for a {@link Task} object. @@ -38,9 +38,9 @@ export class Grouper { public readonly reverse: boolean; - public readonly groupComparator: GroupSorter; + public readonly groupComparator: GroupComparator; - constructor(property: string, grouper: GrouperFunction, reverse: boolean, groupComparator: GroupSorter) { + constructor(property: string, grouper: GrouperFunction, reverse: boolean, groupComparator: GroupComparator) { this.property = property; this.grouper = grouper; this.reverse = reverse; From f61c8193def5d90c74f0f2f97cd0e28c9c42b848 Mon Sep 17 00:00:00 2001 From: Ilyas Landikov Date: Sat, 20 May 2023 19:20:06 +0600 Subject: [PATCH 10/21] fix: toNumber() default priority is 3 --- src/Task.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Task.ts b/src/Task.ts index fdceb2c319..9d74166617 100644 --- a/src/Task.ts +++ b/src/Task.ts @@ -33,11 +33,11 @@ export namespace Priority { case 'Medium': return 2; case 'None': + default: return 3; case 'Low': return 4; } - return 1; } } From 1d29d1dee73f508943107b968398b2db132b6869 Mon Sep 17 00:00:00 2001 From: Ilyas Landikov Date: Sat, 20 May 2023 19:21:56 +0600 Subject: [PATCH 11/21] refactor: remove temp variable --- src/Query/Filter/PriorityField.ts | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/Query/Filter/PriorityField.ts b/src/Query/Filter/PriorityField.ts index 33631d7d29..c01ebd0d9e 100644 --- a/src/Query/Filter/PriorityField.ts +++ b/src/Query/Filter/PriorityField.ts @@ -87,22 +87,18 @@ export class PriorityField extends Field { public grouper(): GrouperFunction { return (task: Task) => { - let priorityName = 'ERROR'; switch (task.priority) { case Priority.High: - priorityName = 'High'; - break; + return ['High']; case Priority.Medium: - priorityName = 'Medium'; - break; + return ['Medium']; case Priority.None: - priorityName = 'None'; - break; + return ['None']; case Priority.Low: - priorityName = 'Low'; - break; + return ['Low']; + default: + return ['ERROR']; } - return [`${priorityName}`]; }; } From bf597ea326b05438d7789caa7c743fa5444b0f07 Mon Sep 17 00:00:00 2001 From: Ilyas Landikov Date: Sun, 21 May 2023 16:44:50 +0600 Subject: [PATCH 12/21] refactor: shorter import type --- src/Query/Filter/PriorityField.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Query/Filter/PriorityField.ts b/src/Query/Filter/PriorityField.ts index c01ebd0d9e..ce5bf1bef2 100644 --- a/src/Query/Filter/PriorityField.ts +++ b/src/Query/Filter/PriorityField.ts @@ -1,7 +1,7 @@ import { Priority, Task } from '../../Task'; import { Explanation } from '../Explain/Explanation'; import type { Comparator } from '../Sorter'; -import type { GroupComparator as GroupComparator, GrouperFunction } from '../Grouper'; +import type { GroupComparator, GrouperFunction } from '../Grouper'; import { Field } from './Field'; import { Filter, FilterOrErrorMessage } from './Filter'; From fefd7b29ffc65e5a557dfb8b0b1e5ff52caca110 Mon Sep 17 00:00:00 2001 From: Ilyas Landikov Date: Sun, 21 May 2023 16:48:41 +0600 Subject: [PATCH 13/21] test: fix test description --- tests/Query/Filter/PriorityField.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Query/Filter/PriorityField.test.ts b/tests/Query/Filter/PriorityField.test.ts index 80e867eeb6..5703b4cd6a 100644 --- a/tests/Query/Filter/PriorityField.test.ts +++ b/tests/Query/Filter/PriorityField.test.ts @@ -176,7 +176,7 @@ describe('grouping by priority', () => { expect(grouper(fromLine({ line: taskLine }))).toEqual(groups); }); - it('task "%s" should have groups: %s', () => { + it('should sort groups according to priority meaning', () => { // Arrange const tasks = [ fromLine({ line: '- [ ] a 🔽' }), From 3180630e24abc803a3c25641536248efffc2367c Mon Sep 17 00:00:00 2001 From: Ilyas Landikov Date: Mon, 22 May 2023 01:21:44 +0600 Subject: [PATCH 14/21] refactor: compare first tasks instead of group names --- src/Query/Filter/Field.ts | 10 ++-------- src/Query/Filter/MultiTextField.ts | 2 +- src/Query/Filter/PriorityField.ts | 8 +------- src/Query/Grouper.ts | 7 +++---- src/Query/TaskGroups.ts | 14 +++----------- src/Task.ts | 16 ---------------- tests/Query/Filter/PriorityField.test.ts | 3 +-- 7 files changed, 11 insertions(+), 49 deletions(-) diff --git a/src/Query/Filter/Field.ts b/src/Query/Filter/Field.ts index 3f109c2b6e..bd7b1299a0 100644 --- a/src/Query/Filter/Field.ts +++ b/src/Query/Filter/Field.ts @@ -1,7 +1,7 @@ import { Sorter } from '../Sorter'; import type { Comparator } from '../Sorter'; import * as RegExpTools from '../../lib/RegExpTools'; -import { type GroupComparator, Grouper } from '../Grouper'; +import { Grouper } from '../Grouper'; import type { GrouperFunction } from '../Grouper'; import type { FilterOrErrorMessage } from './Filter'; @@ -360,7 +360,7 @@ export abstract class Field { * @param reverse - false for normal group order, true for reverse group order. */ public createGrouper(reverse: boolean): Grouper { - return new Grouper(this.fieldNameSingular(), this.grouper(), reverse, this.groupComparator()); + return new Grouper(this.fieldNameSingular(), this.grouper(), reverse, this.comparator()); } /** @@ -382,10 +382,4 @@ export abstract class Field { public createReverseGrouper(): Grouper { return this.createGrouper(true); } - - protected groupComparator(): GroupComparator { - return (groupName1: string, groupName2: string) => { - return groupName1.localeCompare(groupName2, undefined, { numeric: true }); - }; - } } diff --git a/src/Query/Filter/MultiTextField.ts b/src/Query/Filter/MultiTextField.ts index 7f95255863..1115d52fd4 100644 --- a/src/Query/Filter/MultiTextField.ts +++ b/src/Query/Filter/MultiTextField.ts @@ -64,7 +64,7 @@ export abstract class MultiTextField extends TextField { * This overloads {@link Field.createGrouper} to put a plural field name in the {@link Grouper.property}. */ public createGrouper(reverse: boolean): Grouper { - return new Grouper(this.fieldNamePlural(), this.grouper(), reverse, this.groupComparator()); + return new Grouper(this.fieldNamePlural(), this.grouper(), reverse, this.comparator()); } protected grouperRegExp(): RegExp { diff --git a/src/Query/Filter/PriorityField.ts b/src/Query/Filter/PriorityField.ts index ce5bf1bef2..af0dbfa21f 100644 --- a/src/Query/Filter/PriorityField.ts +++ b/src/Query/Filter/PriorityField.ts @@ -1,7 +1,7 @@ import { Priority, Task } from '../../Task'; import { Explanation } from '../Explain/Explanation'; import type { Comparator } from '../Sorter'; -import type { GroupComparator, GrouperFunction } from '../Grouper'; +import type { GrouperFunction } from '../Grouper'; import { Field } from './Field'; import { Filter, FilterOrErrorMessage } from './Filter'; @@ -101,10 +101,4 @@ export class PriorityField extends Field { } }; } - - protected groupComparator(): GroupComparator { - return (groupName1: string, groupName2: string) => { - return Priority.toNumber(groupName1) - Priority.toNumber(groupName2); - }; - } } diff --git a/src/Query/Grouper.ts b/src/Query/Grouper.ts index 36d6af343d..40c4c1e550 100644 --- a/src/Query/Grouper.ts +++ b/src/Query/Grouper.ts @@ -1,4 +1,5 @@ import type { Task } from '../Task'; +import type { Comparator } from './Sorter'; /** * A group-naming function, that takes a Task object and returns zero or more @@ -10,8 +11,6 @@ import type { Task } from '../Task'; */ export type GrouperFunction = (task: Task) => string[]; -export type GroupComparator = (groupName1: string, groupName2: string) => number; - /** * A named function that is used to determine the group heading name(s) to use for a {@link Task} object. * @@ -38,9 +37,9 @@ export class Grouper { public readonly reverse: boolean; - public readonly groupComparator: GroupComparator; + public readonly groupComparator: Comparator; - constructor(property: string, grouper: GrouperFunction, reverse: boolean, groupComparator: GroupComparator) { + constructor(property: string, grouper: GrouperFunction, reverse: boolean, groupComparator: Comparator) { this.property = property; this.grouper = grouper; this.reverse = reverse; diff --git a/src/Query/TaskGroups.ts b/src/Query/TaskGroups.ts index 5ee457ede4..6ffe742dca 100644 --- a/src/Query/TaskGroups.ts +++ b/src/Query/TaskGroups.ts @@ -105,18 +105,10 @@ export class TaskGroups { private sortTaskGroups() { const compareFn = (group1: TaskGroup, group2: TaskGroup) => { - // Compare two TaskGroup objects, sorting them by the group names at each grouping level. - const groupNames1 = group1.groups; - const groupNames2 = group2.groups; - // The containers are guaranteed to be identical sizes, - // they have one value for each 'group by' line in the query. - for (let i = 0; i < groupNames1.length; i++) { - // For now, we only have one sort option: sort by the names of the groups. - // In future, we will add control over the sorting of group headings, - // which will likely involve adjusting this code to sort by applying a Comparator - // to the first Task in each group. + // Compare two TaskGroup objects, sorting them by first task in each group. + for (let i = 0; i < this._groupers.length; i++) { const grouper = this._groupers[i]; - const result = grouper.groupComparator(groupNames1[i], groupNames2[i]); + const result = grouper.groupComparator(group1.tasks[0], group2.tasks[0]); if (result !== 0) { return grouper.reverse ? -result : result; } diff --git a/src/Task.ts b/src/Task.ts index 9d74166617..741fb5d5ff 100644 --- a/src/Task.ts +++ b/src/Task.ts @@ -25,22 +25,6 @@ export enum Priority { Low = '4', } -export namespace Priority { - export function toNumber(priority: string): number { - switch (priority) { - case 'High': - return 1; - case 'Medium': - return 2; - case 'None': - default: - return 3; - case 'Low': - return 4; - } - } -} - export class TaskRegularExpressions { public static readonly dateFormat = 'YYYY-MM-DD'; diff --git a/tests/Query/Filter/PriorityField.test.ts b/tests/Query/Filter/PriorityField.test.ts index 5703b4cd6a..ca008b379b 100644 --- a/tests/Query/Filter/PriorityField.test.ts +++ b/tests/Query/Filter/PriorityField.test.ts @@ -3,13 +3,12 @@ import { TaskBuilder } from '../../TestingTools/TaskBuilder'; import { testFilter } from '../../TestingTools/FilterTestHelpers'; import { PriorityField } from '../../../src/Query/Filter/PriorityField'; import { fromLine } from '../../TestHelpers'; - +import { TaskGroups } from '../../../src/Query/TaskGroups'; import { expectTaskComparesAfter, expectTaskComparesBefore, expectTaskComparesEqual, } from '../../CustomMatchers/CustomMatchersForSorting'; -import { TaskGroups } from '../../../src/Query/TaskGroups'; function testTaskFilterForTaskWithPriority(filter: string, priority: Priority, expected: boolean) { const builder = new TaskBuilder(); From 11c12be2b381db66c65ac6d70d66fb01687bc36f Mon Sep 17 00:00:00 2001 From: Ilyas Landikov Date: Mon, 22 May 2023 11:21:06 +0600 Subject: [PATCH 15/21] docs: update group names in docs --- docs/Queries/Grouping.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/Queries/Grouping.md b/docs/Queries/Grouping.md index c14f7af461..fd41e3584f 100644 --- a/docs/Queries/Grouping.md +++ b/docs/Queries/Grouping.md @@ -77,10 +77,10 @@ For more information, including adding your own customised statuses, see [[Statu 1. `priority` - The priority of the task, namely one of: - - `Priority 1: High` - - `Priority 2: Medium` - - `Priority 3: None` - - `Priority 4: Low` + - `High` + - `Medium` + - `None` + - `Low` 1. `urgency` ([[Urgency|urgency]]) - Currently, the groups run from the lowest urgency to highest. - You can reverse this with `group by urgency reverse`. From b7f9f2025237bc83d7e142beafc27d153538e8b8 Mon Sep 17 00:00:00 2001 From: Ilyas Landikov Date: Mon, 22 May 2023 14:51:19 +0600 Subject: [PATCH 16/21] test: test priority group sorting --- tests/Query/Filter/UrgencyField.test.ts | 33 +++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/Query/Filter/UrgencyField.test.ts b/tests/Query/Filter/UrgencyField.test.ts index d079bc5034..ea9cb6b3e4 100644 --- a/tests/Query/Filter/UrgencyField.test.ts +++ b/tests/Query/Filter/UrgencyField.test.ts @@ -12,6 +12,7 @@ import { expectTaskComparesEqual, } from '../../CustomMatchers/CustomMatchersForSorting'; import { fromLine } from '../../TestHelpers'; +import { TaskGroups } from '../../../src/Query/TaskGroups'; window.moment = moment; @@ -96,4 +97,36 @@ describe('grouping by urgency', () => { // Assert expect(grouper(fromLine({ line: taskLine }))).toEqual(groups); }); + + it('should sort groups from more urgent to less urgent', () => { + // Arrange + const tasks = [ + new TaskBuilder().description('task1').dueDate('2023-05-22').build(), + new TaskBuilder().description('task2').dueDate('2023-05-21').build(), + ]; + + const grouper = [new UrgencyField().createNormalGrouper()]; + const groups = new TaskGroups(grouper, tasks); + + // Assert + expect(groups.toString()).toMatchInlineSnapshot(` + "Groupers (if any): + - urgency + + Group names: [11.21] + #### [urgency] 11.21 + - [ ] task2 📅 2023-05-21 + + --- + + Group names: [10.75] + #### [urgency] 10.75 + - [ ] task1 📅 2023-05-22 + + --- + + 2 tasks + " + `); + }); }); From 67a5baff8c65a545f31891fc07ca2c300ddb53bb Mon Sep 17 00:00:00 2001 From: Ilyas Landikov Date: Mon, 22 May 2023 14:54:15 +0600 Subject: [PATCH 17/21] docs: document updated urgency grouping behaviour --- docs/Queries/Grouping.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/docs/Queries/Grouping.md b/docs/Queries/Grouping.md index fd41e3584f..72c33231f5 100644 --- a/docs/Queries/Grouping.md +++ b/docs/Queries/Grouping.md @@ -82,9 +82,7 @@ For more information, including adding your own customised statuses, see [[Statu - `None` - `Low` 1. `urgency` ([[Urgency|urgency]]) - - Currently, the groups run from the lowest urgency to highest. - - You can reverse this with `group by urgency reverse`. - - In a future release, the default group order will become from the highest urgency to lowest. + - From the highest urgency to lowest. 1. `recurring` - Whether the task is recurring: either `Recurring` or `Not Recurring`. 1. `recurrence` From 65d132ccd0ce8a9f569ce98713f842b06c58ba8d Mon Sep 17 00:00:00 2001 From: Ilyas Landikov Date: Mon, 22 May 2023 15:55:36 +0600 Subject: [PATCH 18/21] refactor: add DescriptionLengthGroupingfield for testing --- .../Filter/DescriptionLengthGroupingField.ts | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 src/Query/Filter/DescriptionLengthGroupingField.ts diff --git a/src/Query/Filter/DescriptionLengthGroupingField.ts b/src/Query/Filter/DescriptionLengthGroupingField.ts new file mode 100644 index 0000000000..39e016e2eb --- /dev/null +++ b/src/Query/Filter/DescriptionLengthGroupingField.ts @@ -0,0 +1,43 @@ +import type { Task } from 'Task'; +import type { GrouperFunction } from 'Query/Grouper'; +import { Field } from './Field'; +import { FilterOrErrorMessage } from './Filter'; + +/** This is a class for test purposes of a Field that supports grouping but not sorting + */ +export class DescriptionLengthGroupingfield extends Field { + protected filterRegExp(): RegExp | null { + throw new Error('No filtering for description length field'); + } + public fieldName(): string { + return 'description length'; + } + + public value(task: Task): number { + return task.description.length; + } + + public createFilterOrErrorMessage(line: string): FilterOrErrorMessage { + return FilterOrErrorMessage.fromError(line, 'description length field does not support filtering'); + } + + // ----------------------------------------------------------------------------------------------------------------- + // Sorting + // ----------------------------------------------------------------------------------------------------------------- + + // Doesn't support sorting by default as in Field.ts + + // ----------------------------------------------------------------------------------------------------------------- + // Grouping + // ----------------------------------------------------------------------------------------------------------------- + + public supportsGrouping(): boolean { + return true; + } + + public grouper(): GrouperFunction { + return (task: Task) => { + return [this.value(task).toString()]; + }; + } +} From 8c2adc38aa3889ec276295e6987a1037cf632d9d Mon Sep 17 00:00:00 2001 From: Ilyas Landikov Date: Mon, 22 May 2023 15:57:08 +0600 Subject: [PATCH 19/21] fix: add defalt group comparator if Field does not support sorting --- src/Query/Filter/Field.ts | 22 ++++++++++++++++++- .../DescriptionLengthGroupingField.test.ts | 8 +++++++ 2 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 tests/Query/Filter/DescriptionLengthGroupingField.test.ts diff --git a/src/Query/Filter/Field.ts b/src/Query/Filter/Field.ts index bd7b1299a0..ee363e1fec 100644 --- a/src/Query/Filter/Field.ts +++ b/src/Query/Filter/Field.ts @@ -1,3 +1,4 @@ +import type { Task } from 'Task'; import { Sorter } from '../Sorter'; import type { Comparator } from '../Sorter'; import * as RegExpTools from '../../lib/RegExpTools'; @@ -360,7 +361,13 @@ export abstract class Field { * @param reverse - false for normal group order, true for reverse group order. */ public createGrouper(reverse: boolean): Grouper { - return new Grouper(this.fieldNameSingular(), this.grouper(), reverse, this.comparator()); + let defaultOrFieldComparator = this.defaultGroupComparator; + + if (this.supportsSorting()) { + defaultOrFieldComparator = this.comparator(); + } + + return new Grouper(this.fieldNameSingular(), this.grouper(), reverse, defaultOrFieldComparator); } /** @@ -382,4 +389,17 @@ export abstract class Field { public createReverseGrouper(): Grouper { return this.createGrouper(true); } + + private defaultGroupComparator: Comparator = (a: Task, b: Task) => { + const groupNamesA = this.grouper()(a); + const groupNamesB = this.grouper()(b); + + for (let i = 0; i < groupNamesA.length; i++) { + // The containers are guaranteed to be identical sizes since we are calling the same grouper + return groupNamesA[i].localeCompare(groupNamesB[i], undefined, { numeric: true }); + } + + // identical if we reach here + return 0; + }; } diff --git a/tests/Query/Filter/DescriptionLengthGroupingField.test.ts b/tests/Query/Filter/DescriptionLengthGroupingField.test.ts new file mode 100644 index 0000000000..b8aced7f92 --- /dev/null +++ b/tests/Query/Filter/DescriptionLengthGroupingField.test.ts @@ -0,0 +1,8 @@ +import { DescriptionLengthGroupingfield } from '../../../src/Query/Filter/DescriptionLengthGroupingField'; + +describe('test a Field class that supports grouping without sorting', () => { + it('should create the grouper', () => { + const grouper = new DescriptionLengthGroupingfield().createNormalGrouper(); + expect(grouper).toBeDefined(); + }); +}); From 75e6cf7e838a32abac32590df4ca1e111c29474f Mon Sep 17 00:00:00 2001 From: Ilyas Landikov Date: Mon, 22 May 2023 16:01:39 +0600 Subject: [PATCH 20/21] test: test default group comparator --- .../DescriptionLengthGroupingField.test.ts | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/Query/Filter/DescriptionLengthGroupingField.test.ts b/tests/Query/Filter/DescriptionLengthGroupingField.test.ts index b8aced7f92..c10327b113 100644 --- a/tests/Query/Filter/DescriptionLengthGroupingField.test.ts +++ b/tests/Query/Filter/DescriptionLengthGroupingField.test.ts @@ -1,8 +1,53 @@ import { DescriptionLengthGroupingfield } from '../../../src/Query/Filter/DescriptionLengthGroupingField'; +import { fromLine } from '../../TestHelpers'; +import { TaskGroups } from '../../../src/Query/TaskGroups'; describe('test a Field class that supports grouping without sorting', () => { it('should create the grouper', () => { const grouper = new DescriptionLengthGroupingfield().createNormalGrouper(); expect(grouper).toBeDefined(); }); + + it('should group in default (alphabetical) order', () => { + const tasks = [ + fromLine({ line: '- [ ] descrip' }), + fromLine({ line: '- [ ] desc' }), + fromLine({ line: '- [ ] description' }), + fromLine({ line: '- [ ] d' }), + ]; + const grouper = [new DescriptionLengthGroupingfield().createNormalGrouper()]; + const groups = new TaskGroups(grouper, tasks); + + expect(groups.toString()).toMatchInlineSnapshot(` + "Groupers (if any): + - description length + + Group names: [1] + #### [description length] 1 + - [ ] d + + --- + + Group names: [4] + #### [description length] 4 + - [ ] desc + + --- + + Group names: [7] + #### [description length] 7 + - [ ] descrip + + --- + + Group names: [11] + #### [description length] 11 + - [ ] description + + --- + + 4 tasks + " + `); + }); }); From a5c389c1d65d33730b25420cc4b1cb3cbece1c09 Mon Sep 17 00:00:00 2001 From: Ilyas Landikov Date: Tue, 23 May 2023 13:06:11 +0600 Subject: [PATCH 21/21] test: fix date to fix urgency calculation --- tests/Query/Filter/UrgencyField.test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/Query/Filter/UrgencyField.test.ts b/tests/Query/Filter/UrgencyField.test.ts index ea9cb6b3e4..620bd3b182 100644 --- a/tests/Query/Filter/UrgencyField.test.ts +++ b/tests/Query/Filter/UrgencyField.test.ts @@ -79,6 +79,15 @@ describe('sorting by urgency', () => { }); describe('grouping by urgency', () => { + beforeAll(() => { + jest.useFakeTimers(); + jest.setSystemTime(new Date(2023, 5 - 1, 22)); + }); + + afterAll(() => { + jest.useRealTimers(); + }); + it('supports grouping methods correctly', () => { expect(new UrgencyField()).toSupportGroupingWithProperty('urgency'); });