Skip to content
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.

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

fix: 'sort by function' now sorts 'true' before 'false' #2597

Merged
merged 3 commits into from
Jan 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 6 additions & 5 deletions docs/Queries/Sorting.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,11 @@ Since Tasks X.Y.Z, **[[Custom Sorting|custom sorting]] by status** is now possib
<!-- placeholder to force blank line before included text --><!-- include: CustomSortingExamples.test.other_properties_task.isDone_docs.approved.md -->

```javascript
sort by function task.isDone
sort by function !task.isDone
```

- Tasks with [[Status Types|Status Type]] `TODO` and `IN_PROGRESS` tasks are sorted before those with types `DONE`, `CANCELLED` and `NON_TASK.
- `sort by function` sorts `true` before `false`
- Hence, we use `!` to negate `task.isDone`, so tasks with [[Status Types|Status Type]] `TODO` and `IN_PROGRESS` tasks are sorted **before** `DONE`, `CANCELLED` and `NON_TASK`.

<!-- placeholder to force blank line after included text --><!-- endInclude -->

Expand Down Expand Up @@ -368,7 +369,7 @@ Since Tasks X.Y.Z, **[[Custom Sorting|custom sorting]] by recurrence** is now po
sort by function task.isRecurring
```

- Sort by whether the task is recurring.
- Sort by whether the task is recurring: recurring tasks will be listed before non-recurring ones.

<!-- placeholder to force blank line after included text --><!-- endInclude -->

Expand Down Expand Up @@ -492,11 +493,11 @@ sort by function task.file.folder
- Enable sorting by the folder containing the task.

```javascript
sort by function reverse task.file.path === query.file.path
sort by function task.file.path === query.file.path
```

- Sort tasks in the same file as the query before tasks in other files.
- **Note**: `false` sort keys sort first, so we `reverse` the result, to get the desired results.
- **Note**: `true` sort keys sort before `false`.

<!-- placeholder to force blank line after included text --><!-- endInclude -->

Expand Down
4 changes: 2 additions & 2 deletions docs/Scripting/Custom Sorting.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,11 @@ sort by function task.file.folder
- Enable sorting by the folder containing the task.

```javascript
sort by function reverse task.file.path === query.file.path
sort by function task.file.path === query.file.path
```

- Sort tasks in the same file as the query before tasks in other files.
- **Note**: `false` sort keys sort first, so we `reverse` the result, to get the desired results.
- **Note**: `true` sort keys sort before `false`.

<!-- placeholder to force blank line after included text --><!-- endInclude -->

Expand Down
15 changes: 5 additions & 10 deletions resources/sample_vaults/Tasks-Demo/Functions/Custom Sorting.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Custom Sorting

This file contains samples for experimenting with 'sort by function'.
This file contains samples for experimenting with `sort by function`.

## Tasks

Expand All @@ -12,19 +12,14 @@ This file contains samples for experimenting with 'sort by function'.
folder includes {{query.file.folder}}

# Put tasks in the same file as the query first.
# Important:
# It's a bit counter-intuitive:
# False sort keys sort first, so we reverse the result, to get the desired results.
sort by function reverse task.file.path === query.file.path
sort by function task.file.path === query.file.path
```

## Error Handling

Demonstrate how errors are handled.
## Demonstrate Error Handling

### Parsing Errors

Errors when reading 'sort by function' instructions.
This section demonstrate how Tasks handles errors when reading `sort by function` instructions.

#### SyntaxError

Expand All @@ -34,7 +29,7 @@ sort by function task.due.formatAsDate(

### Evaluation Errors

Errors when evaluating 'sort by function' instructions during searches.
This section demonstrate how Tasks handles when evaluating `sort by function` instructions during searches.

#### ReferenceError

Expand Down
7 changes: 6 additions & 1 deletion src/Query/Filter/FunctionField.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,12 @@ export class FunctionField extends Field {
return compareByDate(valueA.moment, valueB.moment);
}

// Treat as numeric, so it works well with booleans
if (valueAType === 'boolean') {
// We want true to come before false, as it's been found to give more intuitive behaviour.
// So this is the opposite way round to the calculation below.
return Number(valueB) - Number(valueA);
}

// We use Number() to prevent implicit type conversion, by making the conversion explicit:
const result = Number(valueA) - Number(valueB);
if (isNaN(result)) {
Expand Down
11 changes: 6 additions & 5 deletions tests/Query/Filter/FunctionField.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,10 +226,10 @@ describe('FunctionField - sorting', () => {
expect(checkAndCompareSortKeys(3.15634, 1.535436)).toBeGreaterThan(0);
});

it('should sort false boolean before true', () => {
it('should sort true boolean before false', () => {
expect(checkAndCompareSortKeys(1, 1)).toEqual(SAME);
expect(checkAndCompareSortKeys(false, true)).toEqual(BEFORE);
expect(checkAndCompareSortKeys(true, false)).toEqual(AFTER);
expect(checkAndCompareSortKeys(false, true)).toEqual(AFTER);
expect(checkAndCompareSortKeys(true, false)).toEqual(BEFORE);
});

it('should sort strings case-sensitively and be number-aware', () => {
Expand Down Expand Up @@ -412,15 +412,16 @@ The error message was:
expectTaskComparesAfter(sorter!, fromLine({ line: '* [ ] Hello' }), fromLine({ line: '- [ ] Hello' }));
});

it('sort by function - boolean expression - false sorts before true', () => {
it('sort by function - boolean expression - true sorts before false', () => {
// Arrange
const sorter = new FunctionField().createSorterFromLine('sort by function task.isDone');
const todoTask = fromLine({ line: '- [ ] todo' });
const doneTask = fromLine({ line: '- [x] done' });

// Assert
expect(sorter).not.toBeNull();
expectTaskComparesBefore(sorter!, todoTask, doneTask);
expectTaskComparesAfter(sorter!, todoTask, doneTask);
expectTaskComparesBefore(sorter!, doneTask, todoTask);
});

it('sort by function - using query.file.path', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ sort by function task.file.folder
- Enable sorting by the folder containing the task.

```javascript
sort by function reverse task.file.path === query.file.path
sort by function task.file.path === query.file.path
```

- Sort tasks in the same file as the query before tasks in other files.
- **Note**: `false` sort keys sort first, so we `reverse` the result, to get the desired results.
- **Note**: `true` sort keys sort before `false`.


<!-- placeholder to force blank line after included text -->
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ Enable sorting by the folder containing the task.
====================================================================================


sort by function reverse task.file.path === query.file.path
sort by function task.file.path === query.file.path
Sort tasks in the same file as the query before tasks in other files.
**Note**: `false` sort keys sort first, so we `reverse` the result, to get the desired results.
**Note**: `true` sort keys sort before `false`.
=>
- [ ] xyz in 'a/b.md' in heading 'null'
- [ ] xyz in '' in heading 'heading'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@


```javascript
sort by function task.isDone
sort by function !task.isDone
```

- Tasks with [[Status Types|Status Type]] `TODO` and `IN_PROGRESS` tasks are sorted before those with types `DONE`, `CANCELLED` and `NON_TASK.
- `sort by function` sorts `true` before `false`
- Hence, we use `!` to negate `task.isDone`, so tasks with [[Status Types|Status Type]] `TODO` and `IN_PROGRESS` tasks are sorted **before** `DONE`, `CANCELLED` and `NON_TASK`.


<!-- placeholder to force blank line after included text -->
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ Results of custom sorters



sort by function task.isDone
Tasks with [[Status Types|Status Type]] `TODO` and `IN_PROGRESS` tasks are sorted before those with types `DONE`, `CANCELLED` and `NON_TASK.
sort by function !task.isDone
`sort by function` sorts `true` before `false`
Hence, we use `!` to negate `task.isDone`, so tasks with [[Status Types|Status Type]] `TODO` and `IN_PROGRESS` tasks are sorted **before** `DONE`, `CANCELLED` and `NON_TASK`.
=>
- [ ] Status Todo
- [] Status EMPTY
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
sort by function task.isRecurring
```

- Sort by whether the task is recurring.
- Sort by whether the task is recurring: recurring tasks will be listed before non-recurring ones.


<!-- placeholder to force blank line after included text -->
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@ Results of custom sorters


sort by function task.isRecurring
Sort by whether the task is recurring.
Sort by whether the task is recurring: recurring tasks will be listed before non-recurring ones.
=>
- [ ] my description
- [ ] my description 🔁 every 4 months on the 3rd Wednesday
- [ ] my description 🔁 every month
- [ ] my description 🔁 every month on the 2nd
Expand All @@ -19,5 +18,6 @@ Sort by whether the task is recurring.
- [ ] my description 🔁 every 8 days
- [ ] my description 🔁 every 8 days when done
- [ ] my description 🔁 every day
- [ ] my description
====================================================================================

Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,9 @@ describe('file properties', () => {
// comment to force line break
['sort by function task.file.folder', 'Enable sorting by the folder containing the task'],
[
'sort by function reverse task.file.path === query.file.path',
'sort by function task.file.path === query.file.path',
'Sort tasks in the same file as the query before tasks in other files.',
'**Note**: `false` sort keys sort first, so we `reverse` the result, to get the desired results.',
'**Note**: `true` sort keys sort before `false`.',
],
],
tasks,
Expand Down Expand Up @@ -229,7 +229,12 @@ describe('other properties', () => {

[
'task.isRecurring',
[['sort by function task.isRecurring', 'Sort by whether the task is recurring']],
[
[
'sort by function task.isRecurring',
'Sort by whether the task is recurring: recurring tasks will be listed before non-recurring ones.',
],
],
SampleTasks.withAllRecurrences(),
],

Expand Down Expand Up @@ -302,8 +307,9 @@ describe('other properties', () => {
'task.isDone',
[
[
'sort by function task.isDone',
'Tasks with [[Status Types|Status Type]] `TODO` and `IN_PROGRESS` tasks are sorted before those with types `DONE`, `CANCELLED` and `NON_TASK.',
'sort by function !task.isDone',
'`sort by function` sorts `true` before `false`',
'Hence, we use `!` to negate `task.isDone`, so tasks with [[Status Types|Status Type]] `TODO` and `IN_PROGRESS` tasks are sorted **before** `DONE`, `CANCELLED` and `NON_TASK`.',
],
],
SampleTasks.withAllStatuses(),
Expand Down