Repository: obsidian-tasks-group/obsidian-tasks
Pull request #1411 (2022-12-19 07:40:04)

refactor: TagsField now implements 'sort by tag'
# Description

**Bigger changes**

- `TagsField` now implements sorting
- All original code for sorting individual fields now removed from `Query` and `Sort`
- `Sort`'s only responsibilities now are:
    - Provide the list of default comparators
    - Make a composite comparator

**Supporting changes**

- Add `Field.parseSortLine(line)` and a few supporting methods
    - This allows things like `new TagsField().parseSortLine('sort by tag reverse 2')!`
- Add `Field.fieldNameSingular()`
    - Needed because `TagsField.fieldName()` is `tag/tags`
- Unit tests of `Field` sort-related code added
- Unit tests of `TagsField` sort-related code added

## Motivation and Context

This is step 9 in splitting out all the sort functions to the relevant fields, to make it easier to add new sort instructions, and to be able to re-use the sort code to sort groups in reverse order.

## How has this been tested?

- By running existing tests
- And adding new tests

## Screenshots (if appropriate)

## Types of changes


Internal changes:

- [x] **Refactor** (prefix: `refactor` - non-breaking change which only improves the design or structure of existing code, and making no changes to its external behaviour)
- [x] **Tests** (prefix: `test` - additions and improvements to unit tests and the smoke tests)

## Checklist

- [x] 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](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CONTRIBUTING.md#updating-documentation) accordingly.
- [x] My change has adequate [Unit Test coverage](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CONTRIBUTING.md#maintaining-the-tests).

## Terms

- [x] My contribution follow this project's [contributing guide](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CONTRIBUTING.md)
- [x] I agree to follow this project's [Code of Conduct](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CODE_OF_CONDUCT.md)




Patch:
diff --git a/src/Query/Filter/Field.ts b/src/Query/Filter/Field.ts
index c338269f..c89b53bf 100644
--- a/src/Query/Filter/Field.ts
+++ b/src/Query/Filter/Field.ts
@@ -16,6 +16,10 @@ import type { FilterOrErrorMessage } from './Filter';
  * (such 'no start date' and 'has start date').
  */
 export abstract class Field {
+    // -----------------------------------------------------------------------------------------------------------------
+    // Filtering
+    // -----------------------------------------------------------------------------------------------------------------
+
     /**
      * Returns true if the class can parse the given instruction line.
      *
@@ -76,10 +80,29 @@ export abstract class Field {
      * Return the name of this field, to be used in error messages.
      * This usually matches the instruction name, but does not always
      * (see start and starts).
+     *
+     * Also, some fields have more than one name, separated by '/'.
+     * See {@link TagsField}, for example.
      * @public
+     *
+     * @see fieldNameSingular
      */
     public abstract fieldName(): string;
 
+    /**
+     * Returns the singular form of the field's name.
+     * @public
+     *
+     * @see fieldName
+     */
+    public fieldNameSingular(): string {
+        return this.fieldName();
+    }
+
+    // -----------------------------------------------------------------------------------------------------------------
+    // Sorting
+    // -----------------------------------------------------------------------------------------------------------------
+
     /**
      * Return whether the code for this field implements sorting of tasks
      */
@@ -88,12 +111,97 @@ export abstract class Field {
         return false;
     }
 
+    /**
+     * Parse a 'sort by' line and return a Sorting object.
+     *
+     * Returns null line does not match this field or is invalid,
+     * or this field does not support sorting.
+     */
+    public parseSortLine(line: string): Sorting | null {
+        if (!this.supportsSorting()) {
+            return null;
+        }
+
+        if (!this.canCreateSorterForLine(line)) {
+            return null;
+        }
+
+        const sorting = this.createSorterFromLine(line);
+        if (sorting) {
+            return sorting;
+        }
+        return null;
+    }
+
+    /**
+     * Returns true if the class can parse the given 'sort by' instruction line.
+     *
+     * Current implementation simply checks whether the class does support sorting,
+     * and whether the line matches this.sorterRegExp().
+     * @param line - A line from a ```tasks``` block.
+     *
+     * @see {@link createSorterFromLine}
+     */
+    public canCreateSorterForLine(line: string): boolean {
+        if (!this.supportsSorting()) {
+            return false;
+        }
+
+        return Field.lineMatchesFilter(this.sorterRegExp(), line);
+    }
+
+    /**
+     * Parse the line, and return either a {@link Sorting} object or null.
+     *
+     * This default implementation works for all fields that support
+     * the default sorting pattern of `sort by <fieldName> (reverse)?`.
+     *
+     * Fields that offer more complicated 'sort by' options can override
+     * this method.
+     *
+     * @param line - A 'sort by' line from a ```tasks``` block.
+     *
+     * @see {@link canCreateSorterForLine}
+     */
+    public createSorterFromLine(line: string): Sorting | null {
+        if (!this.supportsSorting()) {
+            return null;
+        }
+
+        const match = Field.getMatch(this.sorterRegExp(), line);
+        if (match === null) {
+            return null;
+        }
+
+        const reverse = !!match[1];
+        return this.createSorter(reverse);
+    }
+
+    /**
+     * Return a regular expression that will match a correctly-formed
+     * instruction line for sorting Tasks by this field.
+     *
+     * Throws if this field does not support sorting.
+     *
+     * `match[1]` will be either `reverse` or undefined.
+     *
+     * Fields that offer more complicated 'sort by' options can override
+     * this method.
+     */
+    protected sorterRegExp(): RegExp {
+        if (!this.supportsSorting()) {
+            throw Error(`sorterRegExp() unimplemented for ${this.fieldNameSingular()}`);
+        }
+
+        return new RegExp(`^sort by ${this.fieldNameSingular()}( reverse)?`);
+    }
+
     /**
      * Return a function to compare two Task objects, for use in sorting by this field's value.
      */
     public comparator(): Comparator {
         // TODO Make abstract
-        throw Error(`comparator() unimplemented for ${this.fieldName()}`);
+        throw Error(`comparator() unimplemented for ${this.fieldNameSingular()}`);
     }
 
     /**
@@ -101,7 +209,7 @@ export abstract class Field {
      * @param reverse - false for normal sort order, true for reverse sort order.
      */
     public createSorter(reverse: boolean): Sorting {
-        return new Sorting(reverse, this.fieldName(), this.comparator());
+        return new Sorting(reverse, this.fieldNameSingular(), this.comparator());
     }
 
     /**
diff --git a/src/Query/Filter/TagsField.ts b/src/Query/Filter/TagsField.ts
index 3a9ae1b0..e8f8ce39 100644
--- a/src/Query/Filter/TagsField.ts
+++ b/src/Query/Filter/TagsField.ts
@@ -1,4 +1,6 @@
 import type { Task } from '../../Task';
+import type { Comparator } from '../Sorting';
+import { Sorting } from '../Sorting';
 import { MultiTextField } from './MultiTextField';
 
 /**
@@ -7,6 +9,10 @@ import { MultiTextField } from './MultiTextField';
  * Tags can be searched for with and without the hash tag at the start.
  */
 export class TagsField extends MultiTextField {
+    // -----------------------------------------------------------------------------------------------------------------
+    // Filtering
+    // -----------------------------------------------------------------------------------------------------------------
+
     public fieldNameSingular(): string {
         return 'tag';
     }
@@ -14,4 +20,80 @@ export class TagsField extends MultiTextField {
     public values(task: Task): string[] {
         return task.tags;
     }
+
+    // -----------------------------------------------------------------------------------------------------------------
+    // Sorting
+    // -----------------------------------------------------------------------------------------------------------------
+
+    public supportsSorting(): boolean {
+        return true;
+    }
+
+    /** Overridden to add support for tag number.
+     *
+     * @param line
+     */
+    public createSorterFromLine(line: string): Sorting | null {
+        const match = line.match(this.sorterRegExp());
+        if (match === null) {
+            return null;
+        }
+
+        const reverse = !!match[1];
+        const propertyInstance = isNaN(+match[2]) ? 1 : +match[2];
+        const comparator = TagsField.makeCompareByTagComparator(propertyInstance);
+        return new Sorting(reverse, this.fieldNameSingular(), comparator);
+    }
+
+    /**
+     * Return a regular expression that will match a correctly-formed
+     * instruction line for sorting Tasks by tag.
+     *
+     * `match[1]` will be either `reverse` or undefined.
+     * `match[2]` will be either the tag number or undefined.
+     */
+    protected sorterRegExp(): RegExp {
+        return /^sort by tag( reverse)?[\s]*(\d+)?/;
+    }
+
+    /**
+     * Create a ${@link Comparator} that sorts by the first tag.
+     */
+    public comparator(): Comparator {
+        return TagsField.makeCompareByTagComparator(1);
+    }
+
+    private static makeCompareByTagComparator(propertyInstance: number): Comparator {
+        return (a: Task, b: Task) => {
+            // If no tags then assume they are equal.
+            if (a.tags.length === 0 && b.tags.length === 0) {
+                return 0;
+            } else if (a.tags.length === 0) {
+                // a is less than b
+                return 1;
+            } else if (b.tags.length === 0) {
+                // b is less than a
+                return -1;
+            }
+
+            // Arrays start at 0 but the users specify a tag starting at 1.
+            const tagInstanceToSortBy = propertyInstance - 1;
+
+            if (a.tags.length < propertyInstance && b.tags.length >= propertyInstance) {
+                return 1;
+            } else if (b.tags.length < propertyInstance && a.tags.length >= propertyInstance) {
+                return -1;
+            } else if (a.tags.length < propertyInstance && b.tags.length < propertyInstance) {
+                return 0;
+            }
+
+            if (a.tags[tagInstanceToSortBy] < b.tags[tagInstanceToSortBy]) {
+                return -1;
+            } else if (a.tags[tagInstanceToSortBy] > b.tags[tagInstanceToSortBy]) {
+                return 1;
+            } else {
+                return 0;
+            }
+        };
+    }
 }
diff --git a/src/Query/FilterParser.ts b/src/Query/FilterParser.ts
index 1f25dbbd..cada02a1 100644
--- a/src/Query/FilterParser.ts
+++ b/src/Query/FilterParser.ts
@@ -47,22 +47,19 @@ export function parseFilter(filterString: string): FilterOrErrorMessage | null {
 
 export function parseSorter(sorterString: string): Sorting | null {
     // New style parsing, using sorting which is done by the Field classes.
-    const sortByRegexp = /^sort by (\S+)( reverse)?/;
-    const fieldMatch = sorterString.match(sortByRegexp);
-    if (fieldMatch === null) {
+
+    // Optimisation: Check whether line begins with 'sort by'
+    const sortByRegexp = /^sort by /;
+    if (sorterString.match(sortByRegexp) === null) {
         return null;
     }
-    const propertyName = fieldMatch[1];
-    const reverse = !!fieldMatch[2];
 
+    // See if any of the fields can parse the line.
     for (const creator of fieldCreators) {
         const field = creator();
-        if (field.fieldName() === propertyName) {
-            if (field.supportsSorting()) {
-                return field.createSorter(reverse);
-            } else {
-                return null;
-            }
+        const sorter = field.parseSortLine(sorterString);
+        if (sorter) {
+            return sorter;
         }
     }
     return null;
diff --git a/src/Query/Query.ts b/src/Query/Query.ts
index bc8c78d7..136b8145 100644
--- a/src/Query/Query.ts
+++ b/src/Query/Query.ts
@@ -9,11 +9,6 @@ import { parseFilter, parseSorter } from './FilterParser';
 import { Group } from './Group';
 import type { Filter } from './Filter/Filter';
 
-// TODO Work through the values in SortingProperty, moving their comparison
-//      functions to the corresponding Field implementations.
-//      Then remove SortingProperty.
-export type SortingProperty = 'tag';
-
 export type GroupingProperty =
     | 'backlink'
     | 'done'
@@ -43,10 +38,6 @@ export class Query implements IQuery {
     private _sorting: Sorting[] = [];
     private _grouping: Grouping[] = [];
 
-    // If a tag is specified the user can also add a number to specify
-    // which one to sort by if there is more than one.
-    private readonly sortByRegexp = /^sort by (tag)( reverse)?[\s]*(\d+)?/;
-
     private readonly groupByRegexp =
         /^group by (backlink|done|due|filename|folder|happens|heading|path|priority|recurrence|recurring|root|scheduled|start|status|tags)/;
 
@@ -77,9 +68,6 @@ export class Query implements IQuery {
                     case this.limitRegexp.test(line):
                         this.parseLimit({ line });
                         break;
-                    case this.sortByRegexp.test(line):
-                        this.parseSortBy({ line });
-                        break;
                     case this.parseSortBy2({ line }):
                         break;
                     case this.groupByRegexp.test(line):
@@ -237,24 +225,6 @@ export class Query implements IQuery {
         return false;
     }
 
-    private parseSortBy({ line }: { line: string }): void {
-        // Old-style parsing, based on all the values in SortingProperty above.
-        // TODO Migrate all the compare functions in Sort over to the
-        //      corresponding fields, so that the block below can be removed.
-        const fieldMatch = line.match(this.sortByRegexp);
-        if (fieldMatch !== null) {
-            this._sorting.push(
-                Sort.makeLegacySorting(
-                    !!fieldMatch[2],
-                    isNaN(+fieldMatch[3]) ? 1 : +fieldMatch[3],
-                    fieldMatch[1] as SortingProperty,
-                ),
-            );
-        } else {
-            this._error = 'do not understand query sorting';
-        }
-    }
-
     private parseGroupBy({ line }: { line: string }): void {
         const fieldMatch = line.match(this.groupByRegexp);
         if (fieldMatch !== null) {
diff --git a/src/Query/Sort.ts b/src/Query/Sort.ts
index 36b8b281..e356a4b1 100644
--- a/src/Query/Sort.ts
+++ b/src/Query/Sort.ts
@@ -1,7 +1,6 @@
 import type { Task } from '../Task';
-import { Sorting } from './Sorting';
 import type { Comparator } from './Sorting';
-import type { Query, SortingProperty } from './Query';
+import type { Query } from './Query';
 // TODO Remove the cyclic dependency between StatusField and Sort.
 import { StatusField } from './Filter/StatusField';
 import { DueDateField } from './Filter/DueDateField';
@@ -29,38 +28,6 @@ export class Sort {
         return tasks.sort(Sort.makeCompositeComparator([...userComparators, ...defaultComparators]));
     }
 
-    /**
-     * Legacy function, for creating a Sorting object for a SortingProperty type.
-     *
-     * TODO Once SortingProperty in Query.ts has been removed, remove this method.
-     * @param reverse - whether the sort order should be reversed.
-     * @param propertyInstance - for tag sorting, this is a 1-based index for the tag number in the task to sort by.
-     *                           TODO eventually, move this number to the comparator used for sorting by tag.
-     * @param property - the name of the property. This string must match
-     *                   one of the values in ${@link SortingProperty}.
-     */
-    public static makeLegacySorting(reverse: boolean, propertyInstance: number, property: SortingProperty): Sorting {
-        const comparator = Sort.makeLegacyComparator(property, propertyInstance);
-        return new Sorting(reverse, property, comparator);
-    }
-
-    /**
-     * Legacy function, for creating a Comparator function for a SortingProperty type.
-     *
-     * TODO Once SortingProperty in Query.ts has been removed, remove this method.
-     * @param property - the name of the property. This string must match
-     *                   one of the values in ${@link SortingProperty}.
-     *                   Throws if property not recognised.
-     * @param propertyInstance - for tag sorting, this is a 1-based index for the tag number in the task to sort by.
-     *                           TODO eventually, move this number to the comparator used for sorting by tag.
-     */
-    public static makeLegacyComparator(property: SortingProperty, propertyInstance: number): Comparator {
-        if (property !== 'tag') {
-            throw Error('Unrecognised legacy sort keyword: ' + property);
-        }
-        return Sort.makeCompareByTagComparator(propertyInstance);
-    }
-
     private static makeCompositeComparator(comparators: Comparator[]): Comparator {
         return (a, b) => {
             for (const comparator of comparators) {
@@ -72,38 +39,4 @@ export class Sort {
             return 0;
         };
     }
-
-    private static makeCompareByTagComparator(propertyInstance: number): Comparator {
-        return (a: Task, b: Task) => {
-            // If no tags then assume they are equal.
-            if (a.tags.length === 0 && b.tags.length === 0) {
-                return 0;
-            } else if (a.tags.length === 0) {
-                // a is less than b
-                return 1;
-            } else if (b.tags.length === 0) {
-                // b is less than a
-                return -1;
-            }
-
-            // Arrays start at 0 but the users specify a tag starting at 1.
-            const tagInstanceToSortBy = propertyInstance - 1;
-
-            if (a.tags.length < propertyInstance && b.tags.length >= propertyInstance) {
-                return 1;
-            } else if (b.tags.length < propertyInstance && a.tags.length >= propertyInstance) {
-                return -1;
-            } else if (a.tags.length < propertyInstance && b.tags.length < propertyInstance) {
-                return 0;
-            }
-
-            if (a.tags[tagInstanceToSortBy] < b.tags[tagInstanceToSortBy]) {
-                return -1;
-            } else if (a.tags[tagInstanceToSortBy] > b.tags[tagInstanceToSortBy]) {
-                return 1;
-            } else {
-                return 0;
-            }
-        };
-    }
 }


Test patch:
diff --git a/tests/Query/Filter/Field.test.ts b/tests/Query/Filter/Field.test.ts
new file mode 100644
index 00000000..7613a4bf
--- /dev/null
+++ b/tests/Query/Filter/Field.test.ts
@@ -0,0 +1,109 @@
+import { Field } from '../../../src/Query/Filter/Field';
+import type { FilterOrErrorMessage } from '../../../src/Query/Filter/Filter';
+import type { Comparator } from '../../../src/Query/Sorting';
+import type { Task } from '../../../src/Task';
+import { expectTaskComparesBefore } from '../../CustomMatchers/CustomMatchersForSorting';
+import { TaskBuilder } from '../../TestingTools/TaskBuilder';
+
+class TestFieldSortingUnSupported extends Field {
+    // Filtering
+    createFilterOrErrorMessage(_line: string): FilterOrErrorMessage {
+        throw Error(`createFilterOrErrorMessage() unimplemented for ${this.fieldName()}`);
+    }
+
+    fieldName(): string {
+        return 'unsupported';
+    }
+
+    protected filterRegExp(): RegExp | null {
+        throw Error(`filterRegExp() unimplemented for ${this.fieldName()}`);
+    }
+}
+
+class DescriptionLengthField extends Field {
+    // Filtering
+    createFilterOrErrorMessage(_line: string): FilterOrErrorMessage {
+        throw Error(`createFilterOrErrorMessage() unimplemented for ${this.fieldName()}`);
+    }
+
+    fieldName(): string {
+        return 'description-length';
+    }
+
+    protected filterRegExp(): RegExp | null {
+        throw Error(`filterRegExp() unimplemented for ${this.fieldName()}`);
+    }
+
+    // Sorting
+    supportsSorting(): boolean {
+        return true;
+    }
+
+    comparator(): Comparator {
+        return (a: Task, b: Task) => {
+            return a.description.length - b.description.length;
+        };
+    }
+}
+
+describe('sorting - base class usability and implementation', () => {
+    describe('field not supporting sorting', () => {
+        const unsupported = new TestFieldSortingUnSupported();
+
+        it('should not support sorting', () => {
+            expect(unsupported.supportsSorting()).toEqual(false);
+        });
+
+        it('should not create a comparator', () => {
+            const t = () => {
+                unsupported.comparator();
+            };
+            expect(t).toThrow(Error);
+        });
+
+        it('should fail to parse a "valid" line', () => {
+            // expect(unsupported.parseSortLine('sort by unsupported')).toThrow(Error);
+            const line = 'sort by unsupported';
+            expect(unsupported.canCreateSorterForLine(line)).toBe(false);
+            expect(unsupported.createSorterFromLine(line)).toBeNull();
+            const sorting = unsupported.parseSortLine(line);
+            expect(sorting).toBeNull();
+        });
+    });
+
+    describe('field supporting sorting', () => {
+        const supported = new DescriptionLengthField();
+
+        it('should support sorting', () => {
+            expect(supported.supportsSorting()).toEqual(true);
+        });
+
+        it('should create a comparator', () => {
+            expect(supported.comparator()).not.toBeNull();
+        });
+
+        it('should parse a valid line', () => {
+            const line = 'sort by description-length';
+            expect(supported.canCreateSorterForLine(line)).toBe(true);
+            expect(supported.createSorterFromLine(line)).not.toBeNull();
+            const sorting = supported.parseSortLine(line);
+            expect(sorting).not.toBeNull();
+            expect(sorting?.property).toEqual('description-length');
+        });
+
+        it('should fail to parse a invalid line', () => {
+            const line = 'sort by jsdajhasdfa';
+            expect(supported.canCreateSorterForLine(line)).toBe(false);
+            expect(supported.createSorterFromLine(line)).toBeNull();
+            const sorting = supported.parseSortLine(line);
+            expect(sorting).toBeNull();
+        });
+
+        it('should compare two tasks', () => {
+            const sorting = supported.parseSortLine('sort by description-length');
+            const a = new TaskBuilder().description('short description').build();
+            const b = new TaskBuilder().description('very looooooooong description').build();
+            expectTaskComparesBefore(sorting!, a, b);
+        });
+    });
+});
diff --git a/tests/Query/Filter/TagsField.test.ts b/tests/Query/Filter/TagsField.test.ts
index f82e0dad..f689980c 100644
--- a/tests/Query/Filter/TagsField.test.ts
+++ b/tests/Query/Filter/TagsField.test.ts
@@ -6,6 +6,8 @@ import { TagsField } from '../../../src/Query/Filter/TagsField';
 import { DescriptionField } from '../../../src/Query/Filter/DescriptionField';
 import { fromLine } from '../../TestHelpers';
 import { Sort } from '../../../src/Query/Sort';
+import { TaskBuilder } from '../../TestingTools/TaskBuilder';
+import { expectTaskComparesAfter, expectTaskComparesBefore } from '../../CustomMatchers/CustomMatchersForSorting';
 
 expect.extend({
     toMatchTaskFromLine,
@@ -276,8 +278,64 @@ describe('tag/tags', () => {
  * There is also a task with additional characters in the name to ensure it is seen
  * as bigger that one with the same initial characters.
  */
-// TODO Replace this with something simpler but equivalent.
 describe('Sort by tags', () => {
+    describe('field supports sorting', () => {
+        const tagsField = new TagsField();
+
+        it('should support sorting', () => {
+            expect(tagsField.supportsSorting()).toEqual(true);
+        });
+
+        it('should report whether it can parse lines', () => {
+            // Valid sort by tag lines:
+            expect(tagsField.canCreateSorterForLine('sort by tag')).toBe(true);
+            expect(tagsField.canCreateSorterForLine('sort by tag 2')).toBe(true);
+            expect(tagsField.canCreateSorterForLine('sort by tag reverse')).toBe(true);
+            expect(tagsField.canCreateSorterForLine('sort by tag reverse 3')).toBe(true);
+
+            // Invalid lines:
+            expect(tagsField.canCreateSorterForLine('sort by description')).toBe(false);
+        });
+
+        const tag_a = new TaskBuilder().tags(['#a']).build();
+        const tag_b = new TaskBuilder().tags(['#b']).build();
+        const tags_a_b = new TaskBuilder().tags(['#a', '#b']).build();
+        const tags_a_c = new TaskBuilder().tags(['#b', '#c']).build();
+        const tags_a_d = new TaskBuilder().tags(['#a', '#d']).build();
+
+        it('should create a default comparator, sorting by first tag', () => {
+            const comparator = tagsField.comparator();
+            expect(comparator(tags_a_d, tags_a_c)).toBeLessThan(0);
+        });
+
+        it('should parse a valid line with default tag number', () => {
+            const sorter = tagsField.parseSortLine('sort by tag');
+            expect(sorter?.property).toEqual('tag');
+            expect(sorter?.comparator(tag_a, tag_b)).toBeLessThan(0);
+            expectTaskComparesBefore(sorter!, tag_a, tag_b);
+        });
+
+        it('should parse a valid line with a non-default tag number', () => {
+            const sorter = tagsField.parseSortLine('sort by tag 2');
+            expect(sorter?.property).toEqual('tag');
+            expectTaskComparesBefore(sorter!, tags_a_b, tags_a_c);
+        });
+
+        it('should parse a valid line with reverse and a non-default tag number', () => {
+            const sorter = tagsField.parseSortLine('sort by tag reverse 2');
+            expect(sorter?.property).toEqual('tag');
+            expectTaskComparesAfter(sorter!, tags_a_b, tags_a_c);
+        });
+
+        it('should fail to parse a invalid line', () => {
+            const line = 'sort by jsdajhasdfa';
+            expect(tagsField.canCreateSorterForLine(line)).toBe(false);
+            expect(tagsField.createSorterFromLine(line)).toBeNull();
+            const sorting = tagsField.parseSortLine(line);
+            expect(sorting).toBeNull();
+        });
+    });
+
     it('should sort correctly by tag defaulting to first with no global filter', () => {
         // Arrange
         const t1 = fromLine({ line: '- [ ] a #aaa #jjj' });
@@ -296,7 +354,7 @@ describe('Sort by tags', () => {
         expect(
             Sort.by(
                 {
-                    sorting: [Sort.makeLegacySorting(false, 1, 'tag')],
+                    sorting: [new TagsField().parseSortLine('sort by tag 1')!],
                 },
                 [t1, t3, t5, t7, t6, t4, t2, t8, t9, t10],
             ),
@@ -321,7 +379,7 @@ describe('Sort by tags', () => {
         expect(
             Sort.by(
                 {
-                    sorting: [Sort.makeLegacySorting(true, 1, 'tag')],
+                    sorting: [new TagsField().parseSortLine('sort by tag reverse 1')!],
                 },
                 [t1, t3, t5, t7, t6, t4, t2, t8, t9, t10],
             ),
@@ -338,7 +396,7 @@ describe('Sort by tags', () => {
         expect(
             Sort.by(
                 {
-                    sorting: [Sort.makeLegacySorting(false, 2, 'tag')],
+                    sorting: [new TagsField().parseSortLine('sort by tag 2')!],
                 },
                 [t4, t3, t2, t1, t5],
             ),
@@ -355,7 +413,7 @@ describe('Sort by tags', () => {
         expect(
             Sort.by(
                 {
-                    sorting: [Sort.makeLegacySorting(true, 2, 'tag')],
+                    sorting: [new TagsField().parseSortLine('sort by tag reverse 2')!],
                 },
                 [t4, t3, t2, t1, t5],
             ),
@@ -386,7 +444,7 @@ describe('Sort by tags', () => {
         expect(
             Sort.by(
                 {
-                    sorting: [Sort.makeLegacySorting(false, 1, 'tag')],
+                    sorting: [new TagsField().parseSortLine('sort by tag 1')!],
                 },
                 [t1, t12, t3, t13, t5, t7, t6, t4, t2, t8, t9, t10, t11],
             ),
@@ -420,7 +478,7 @@ describe('Sort by tags', () => {
         expect(
             Sort.by(
                 {
-                    sorting: [Sort.makeLegacySorting(true, 1, 'tag')],
+                    sorting: [new TagsField().parseSortLine('sort by tag reverse 1')!],
                 },
                 [t1, t12, t3, t13, t5, t7, t6, t4, t2, t8, t9, t10, t11],
             ),
@@ -447,7 +505,7 @@ describe('Sort by tags', () => {
         // Act
         const result = Sort.by(
             {
-                sorting: [Sort.makeLegacySorting(false, 2, 'tag')],
+                sorting: [new TagsField().parseSortLine('sort by tag 2')!],
             },
             [t4, t7, t5, t2, t3, t1, t8, t6],
         );
@@ -476,7 +534,7 @@ describe('Sort by tags', () => {
         // Act
         const result = Sort.by(
             {
-                sorting: [Sort.makeLegacySorting(true, 2, 'tag')],
+                sorting: [new TagsField().parseSortLine('sort by tag reverse 2')!],
             },
             [t4, t7, t5, t2, t3, t1, t8, t6],
         );
@@ -516,8 +574,8 @@ describe('Sort by tags', () => {
             Sort.by(
                 {
                     sorting: [
-                        Sort.makeLegacySorting(false, 2, 'tag'), // tag 2 - ascending
-                        Sort.makeLegacySorting(false, 1, 'tag'), // tag 1 - ascending
+                        new TagsField().parseSortLine('sort by tag 2')!, // tag 2 - ascending
+                        new TagsField().parseSortLine('sort by tag 1')!, // tag 1 - ascending
                         new DescriptionField().createNormalSorter(), // then description - ascending
                     ],
                 },