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: Allow spaces after show and hide - and improve error message #3213

Merged
merged 31 commits into from
Dec 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
2c0ffeb
refactor: - Remove unnecessary initialisation of Query._queryId
claremacrae Dec 1, 2024
0e98269
refactor: . Make some fields in Query readonly - done by SonarLint
claremacrae Dec 1, 2024
53483bd
comment: Remove incorrect comments - 'show/hide tree' is now released
claremacrae Dec 1, 2024
ff0de4d
refactor: - Use early-return in Query.parseHideOptions()
claremacrae Dec 1, 2024
67e6c1f
refactor: . Reorder cases in switch statement to group together relat…
claremacrae Dec 1, 2024
a782dad
test: - Test recognition of show/hide backlinks (plural)
claremacrae Dec 1, 2024
d51471e
refactor: . Replace switch with if
claremacrae Dec 1, 2024
0bb3682
refactor: . Unwrap else statements
claremacrae Dec 1, 2024
1e16bf3
refactor: Introduce expectedErrorMessage parameter to isInvalidQueryI…
claremacrae Dec 1, 2024
bcdca33
refactor: - Move optional parameter up to isInvalidQueryInstructionLo…
claremacrae Dec 1, 2024
415f879
refactor: - Add optional expectedErrorMessage to isInvalidQueryInstru…
claremacrae Dec 1, 2024
86e5628
test: - Demonstrate that show/hide do not allow extra spaces currently
claremacrae Dec 1, 2024
6ee8017
refactor: - Simplify addition of new show/hide options, simplifying a…
claremacrae Dec 1, 2024
265d2b1
refactor: . flip if/else
claremacrae Dec 1, 2024
d421f25
refactor: - Extract parseTaskShowHideOptions()
claremacrae Dec 1, 2024
88f3c84
refactor: . Extract variable visible
claremacrae Dec 1, 2024
197d6fe
refactor: . Replace hide parameter with visible
claremacrae Dec 1, 2024
43b4a22
jsdoc: Improve wording
claremacrae Dec 1, 2024
105147c
refactor: - Extract parseQueryShowHideOptions()
claremacrae Dec 1, 2024
991978e
refactor: . Extract queryLayoutOptions
claremacrae Dec 1, 2024
648997b
refactor: . Extract parseQueryShowHideOptions()
claremacrae Dec 1, 2024
d377e38
refactor: . Move parseQueryShowHideOptions() to QueryLayoutOptions.ts
claremacrae Dec 1, 2024
602c57a
jsdoc: Move parseQueryShowHideOptions() docs to where the implementat…
claremacrae Dec 1, 2024
d29c8cb
refactor: . Inline Query.parseQueryShowHideOptions()
claremacrae Dec 1, 2024
6a06d94
refactor: . Extract taskLayoutOptions
claremacrae Dec 1, 2024
6a7637b
refactor: . Extract parseTaskShowHideOptions()
claremacrae Dec 1, 2024
26d35e4
refactor: . Fix parameter order in parseTaskShowHideOptions()
claremacrae Dec 1, 2024
e222404
jsdoc: Move parseTaskShowHideOptions() docs to where the implementati…
claremacrae Dec 1, 2024
b4a7088
jsdoc: Move parseTaskShowHideOptions() TaskLayoutOptions.ts
claremacrae Dec 1, 2024
05aed75
refactor: . Inline Query.parseTaskShowHideOptions()
claremacrae Dec 1, 2024
1f6850b
jsdoc: Add links to related functions
claremacrae Dec 1, 2024
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
38 changes: 37 additions & 1 deletion src/Layout/QueryLayoutOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,43 @@ export class QueryLayoutOptions {
hideBacklinks: boolean = false;
hideEditButton: boolean = false;
hideUrgency: boolean = true;
hideTree: boolean = true; // WARNING: undocumented, and not yet ready for release.
hideTree: boolean = true;
shortMode: boolean = false;
explainQuery: boolean = false;
}

/**
* Parse show/hide options for Query Layout options
* @param queryLayoutOptions
* @param option - must already have been lower-cased
* @param hide - whether the option should be hidden
* @return True if the option was recognised, and false otherwise
* @see parseTaskShowHideOptions
*/
export function parseQueryShowHideOptions(queryLayoutOptions: QueryLayoutOptions, option: string, hide: boolean) {
if (option.startsWith('tree')) {
queryLayoutOptions.hideTree = hide;
return true;
}
if (option.startsWith('task count')) {
queryLayoutOptions.hideTaskCount = hide;
return true;
}
if (option.startsWith('backlink')) {
queryLayoutOptions.hideBacklinks = hide;
return true;
}
if (option.startsWith('postpone button')) {
queryLayoutOptions.hidePostponeButton = hide;
return true;
}
if (option.startsWith('edit button')) {
queryLayoutOptions.hideEditButton = hide;
return true;
}
if (option.startsWith('urgency')) {
queryLayoutOptions.hideUrgency = hide;
return true;
}
return false;
}
60 changes: 60 additions & 0 deletions src/Layout/TaskLayoutOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,63 @@ export class TaskLayoutOptions {
this.setTagsVisibility(!this.areTagsShown());
}
}

/**
* Parse show/hide options for Task layout options
* @param taskLayoutOptions
* @param option - must already have been lower-cased
* @param visible - whether the option should be shown
* @return True if the option was recognised, and false otherwise
* @see parseQueryShowHideOptions
*/
export function parseTaskShowHideOptions(taskLayoutOptions: TaskLayoutOptions, option: string, visible: boolean) {
if (option.startsWith('priority')) {
taskLayoutOptions.setVisibility(TaskLayoutComponent.Priority, visible);
return true;
}
if (option.startsWith('cancelled date')) {
taskLayoutOptions.setVisibility(TaskLayoutComponent.CancelledDate, visible);
return true;
}
if (option.startsWith('created date')) {
taskLayoutOptions.setVisibility(TaskLayoutComponent.CreatedDate, visible);
return true;
}
if (option.startsWith('start date')) {
taskLayoutOptions.setVisibility(TaskLayoutComponent.StartDate, visible);
return true;
}
if (option.startsWith('scheduled date')) {
taskLayoutOptions.setVisibility(TaskLayoutComponent.ScheduledDate, visible);
return true;
}
if (option.startsWith('due date')) {
taskLayoutOptions.setVisibility(TaskLayoutComponent.DueDate, visible);
return true;
}
if (option.startsWith('done date')) {
taskLayoutOptions.setVisibility(TaskLayoutComponent.DoneDate, visible);
return true;
}
if (option.startsWith('recurrence rule')) {
taskLayoutOptions.setVisibility(TaskLayoutComponent.RecurrenceRule, visible);
return true;
}
if (option.startsWith('tags')) {
taskLayoutOptions.setTagsVisibility(visible);
return true;
}
if (option.startsWith('id')) {
taskLayoutOptions.setVisibility(TaskLayoutComponent.Id, visible);
return true;
}
if (option.startsWith('depends on')) {
taskLayoutOptions.setVisibility(TaskLayoutComponent.DependsOn, visible);
return true;
}
if (option.startsWith('on completion')) {
taskLayoutOptions.setVisibility(TaskLayoutComponent.OnCompletion, visible);
return true;
}
return false;
}
94 changes: 21 additions & 73 deletions src/Query/Query.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { getSettings } from '../Config/Settings';
import type { IQuery } from '../IQuery';
import { QueryLayoutOptions } from '../Layout/QueryLayoutOptions';
import { TaskLayoutComponent, TaskLayoutOptions } from '../Layout/TaskLayoutOptions';
import { QueryLayoutOptions, parseQueryShowHideOptions } from '../Layout/QueryLayoutOptions';
import { TaskLayoutOptions, parseTaskShowHideOptions } from '../Layout/TaskLayoutOptions';
import { errorMessageForException } from '../lib/ExceptionTools';
import { logging } from '../lib/logging';
import { expandPlaceholders } from '../Scripting/ExpandPlaceholders';
Expand All @@ -27,24 +27,23 @@ export class Query implements IQuery {

private _limit: number | undefined = undefined;
private _taskGroupLimit: number | undefined = undefined;
private _taskLayoutOptions: TaskLayoutOptions = new TaskLayoutOptions();
private _queryLayoutOptions: QueryLayoutOptions = new QueryLayoutOptions();
private _filters: Filter[] = [];
private readonly _taskLayoutOptions: TaskLayoutOptions = new TaskLayoutOptions();
private readonly _queryLayoutOptions: QueryLayoutOptions = new QueryLayoutOptions();
private readonly _filters: Filter[] = [];
private _error: string | undefined = undefined;
private _sorting: Sorter[] = [];
private _grouping: Grouper[] = [];
private readonly _sorting: Sorter[] = [];
private readonly _grouping: Grouper[] = [];
private _ignoreGlobalQuery: boolean = false;

private readonly hideOptionsRegexp =
/^(hide|show) (task count|backlink|priority|cancelled date|created date|start date|scheduled date|done date|due date|recurrence rule|edit button|postpone button|urgency|tags|depends on|id|on completion|tree)/i;
private readonly hideOptionsRegexp = /^(hide|show) +(.*)/i;
private readonly shortModeRegexp = /^short/i;
private readonly fullModeRegexp = /^full/i;
private readonly explainQueryRegexp = /^explain/i;
private readonly ignoreGlobalQueryRegexp = /^ignore global query/i;

logger = logging.getLogger('tasks.Query');
// Used internally to uniquely log each query execution in the console.
private _queryId: string = '';
private readonly _queryId: string;

private readonly limitRegexp = /^limit (groups )?(to )?(\d+)( tasks?)?/i;

Expand Down Expand Up @@ -304,70 +303,19 @@ ${statement.explainStatement(' ')}

private parseHideOptions(line: string): void {
const hideOptionsMatch = line.match(this.hideOptionsRegexp);
if (hideOptionsMatch !== null) {
const hide = hideOptionsMatch[1].toLowerCase() === 'hide';
const option = hideOptionsMatch[2].toLowerCase();

switch (option) {
case 'tree':
// WARNING: undocumented, and not yet ready for release.
this._queryLayoutOptions.hideTree = hide;
break;
case 'task count':
this._queryLayoutOptions.hideTaskCount = hide;
break;
case 'backlink':
this._queryLayoutOptions.hideBacklinks = hide;
break;
case 'postpone button':
this._queryLayoutOptions.hidePostponeButton = hide;
break;
case 'priority':
this._taskLayoutOptions.setVisibility(TaskLayoutComponent.Priority, !hide);
break;
case 'cancelled date':
this._taskLayoutOptions.setVisibility(TaskLayoutComponent.CancelledDate, !hide);
break;
case 'created date':
this._taskLayoutOptions.setVisibility(TaskLayoutComponent.CreatedDate, !hide);
break;
case 'start date':
this._taskLayoutOptions.setVisibility(TaskLayoutComponent.StartDate, !hide);
break;
case 'scheduled date':
this._taskLayoutOptions.setVisibility(TaskLayoutComponent.ScheduledDate, !hide);
break;
case 'due date':
this._taskLayoutOptions.setVisibility(TaskLayoutComponent.DueDate, !hide);
break;
case 'done date':
this._taskLayoutOptions.setVisibility(TaskLayoutComponent.DoneDate, !hide);
break;
case 'recurrence rule':
this._taskLayoutOptions.setVisibility(TaskLayoutComponent.RecurrenceRule, !hide);
break;
case 'edit button':
this._queryLayoutOptions.hideEditButton = hide;
break;
case 'urgency':
this._queryLayoutOptions.hideUrgency = hide;
break;
case 'tags':
this._taskLayoutOptions.setTagsVisibility(!hide);
break;
case 'id':
this._taskLayoutOptions.setVisibility(TaskLayoutComponent.Id, !hide);
break;
case 'depends on':
this._taskLayoutOptions.setVisibility(TaskLayoutComponent.DependsOn, !hide);
break;
case 'on completion':
this._taskLayoutOptions.setVisibility(TaskLayoutComponent.OnCompletion, !hide);
break;
default:
this.setError('do not understand hide/show option', new Statement(line, line));
}
if (hideOptionsMatch === null) {
return;
}
const hide = hideOptionsMatch[1].toLowerCase() === 'hide';
const option = hideOptionsMatch[2].toLowerCase();

if (parseQueryShowHideOptions(this._queryLayoutOptions, option, hide)) {
return;
}
if (parseTaskShowHideOptions(this._taskLayoutOptions, option, !hide)) {
return;
}
this.setError('do not understand hide/show option', new Statement(line, line));
}

private parseFilter(line: string, statement: Statement) {
Expand Down
34 changes: 28 additions & 6 deletions tests/Query/Query.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,22 @@ function isValidQueryGroup(filter: string) {
lowerCaseFilterGaveExpectionInstruction(filter, query.grouping[0].instruction);
}

function isInvalidQueryInstruction(getQueryError: (source: string) => string | undefined, source: string) {
expect(getQueryError(source)).toEqual(`do not understand query
function isInvalidQueryInstruction(
getQueryError: (source: string) => string | undefined,
source: string,
expectedErrorMessage: string,
) {
expect(getQueryError(source)).toEqual(`${expectedErrorMessage}
Problem line: "${source}"`);
}

function isInvalidQueryInstructionLowerAndUpper(getQueryError: (source: string) => string | undefined, source: string) {
isInvalidQueryInstruction(getQueryError, source);
isInvalidQueryInstruction(getQueryError, source.toUpperCase());
function isInvalidQueryInstructionLowerAndUpper(
getQueryError: (source: string) => string | undefined,
source: string,
expectedErrorMessage: string = 'do not understand query',
) {
isInvalidQueryInstruction(getQueryError, source, expectedErrorMessage);
isInvalidQueryInstruction(getQueryError, source.toUpperCase(), expectedErrorMessage);
}

describe('Query parsing', () => {
Expand Down Expand Up @@ -452,6 +460,7 @@ describe('Query parsing', () => {
'full',
'full mode',
'hide backlink',
'hide backlinks',
'hide cancelled date',
'hide created date',
'hide depends on',
Expand All @@ -476,6 +485,7 @@ describe('Query parsing', () => {
'short',
'short mode',
'show backlink',
'show backlinks',
'show cancelled date',
'show created date',
'show depends on',
Expand Down Expand Up @@ -527,6 +537,18 @@ describe('Query parsing', () => {
});
});

it('should allow spaces between show or hide and a Query option', () => {
const query1 = new Query('show tree');
expect(query1.queryLayoutOptions.hideTree).toBe(false);
expect(query1.error).toBeUndefined();
});

it('should allow spaces between show or hide and a Task option', () => {
const query1 = new Query('hide priority');
expect(query1.taskLayoutOptions.isShown(TaskLayoutComponent.Priority)).toBe(false);
expect(query1.error).toBeUndefined();
});

it('should parse ambiguous sort by queries correctly', () => {
expect(new Query('sort by status').sorting[0].property).toEqual('status');
expect(new Query('SORT BY STATUS').sorting[0].property).toEqual('status');
Expand Down Expand Up @@ -615,7 +637,7 @@ Problem line: "${source}"`,

it('for invalid hide', () => {
const source = 'hide nonsense';
isInvalidQueryInstructionLowerAndUpper(getQueryError, source);
isInvalidQueryInstructionLowerAndUpper(getQueryError, source, 'do not understand hide/show option');
});

it('for unknown instruction', () => {
Expand Down