Skip to content

Commit 4483c27

Browse files
claremacraeclaremacrae
authored
Merge pull request #3213 from obsidian-tasks-group/refactor-query-code
fix: Allow spaces after `show` and `hide` - and improve error message
2 parents 18478dd + 1f6850b commit 4483c27

File tree

4 files changed

+146
-80
lines changed

4 files changed

+146
-80
lines changed

‎src/Layout/QueryLayoutOptions.ts‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,43 @@ export class QueryLayoutOptions {
99
hideBacklinks: boolean = false;
1010
hideEditButton: boolean = false;
1111
hideUrgency: boolean = true;
12-
hideTree: boolean = true; // WARNING: undocumented, and not yet ready for release.
12+
hideTree: boolean = true;
1313
shortMode: boolean = false;
1414
explainQuery: boolean = false;
1515
}
16+
17+
/**
18+
* Parse show/hide options for Query Layout options
19+
* @param queryLayoutOptions
20+
* @param option - must already have been lower-cased
21+
* @param hide - whether the option should be hidden
22+
* @return True if the option was recognised, and false otherwise
23+
* @see parseTaskShowHideOptions
24+
*/
25+
export function parseQueryShowHideOptions(queryLayoutOptions: QueryLayoutOptions, option: string, hide: boolean) {
26+
if (option.startsWith('tree')) {
27+
queryLayoutOptions.hideTree = hide;
28+
return true;
29+
}
30+
if (option.startsWith('task count')) {
31+
queryLayoutOptions.hideTaskCount = hide;
32+
return true;
33+
}
34+
if (option.startsWith('backlink')) {
35+
queryLayoutOptions.hideBacklinks = hide;
36+
return true;
37+
}
38+
if (option.startsWith('postpone button')) {
39+
queryLayoutOptions.hidePostponeButton = hide;
40+
return true;
41+
}
42+
if (option.startsWith('edit button')) {
43+
queryLayoutOptions.hideEditButton = hide;
44+
return true;
45+
}
46+
if (option.startsWith('urgency')) {
47+
queryLayoutOptions.hideUrgency = hide;
48+
return true;
49+
}
50+
return false;
51+
}

‎src/Layout/TaskLayoutOptions.ts‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,3 +93,63 @@ export class TaskLayoutOptions {
9393
this.setTagsVisibility(!this.areTagsShown());
9494
}
9595
}
96+
97+
/**
98+
* Parse show/hide options for Task layout options
99+
* @param taskLayoutOptions
100+
* @param option - must already have been lower-cased
101+
* @param visible - whether the option should be shown
102+
* @return True if the option was recognised, and false otherwise
103+
* @see parseQueryShowHideOptions
104+
*/
105+
export function parseTaskShowHideOptions(taskLayoutOptions: TaskLayoutOptions, option: string, visible: boolean) {
106+
if (option.startsWith('priority')) {
107+
taskLayoutOptions.setVisibility(TaskLayoutComponent.Priority, visible);
108+
return true;
109+
}
110+
if (option.startsWith('cancelled date')) {
111+
taskLayoutOptions.setVisibility(TaskLayoutComponent.CancelledDate, visible);
112+
return true;
113+
}
114+
if (option.startsWith('created date')) {
115+
taskLayoutOptions.setVisibility(TaskLayoutComponent.CreatedDate, visible);
116+
return true;
117+
}
118+
if (option.startsWith('start date')) {
119+
taskLayoutOptions.setVisibility(TaskLayoutComponent.StartDate, visible);
120+
return true;
121+
}
122+
if (option.startsWith('scheduled date')) {
123+
taskLayoutOptions.setVisibility(TaskLayoutComponent.ScheduledDate, visible);
124+
return true;
125+
}
126+
if (option.startsWith('due date')) {
127+
taskLayoutOptions.setVisibility(TaskLayoutComponent.DueDate, visible);
128+
return true;
129+
}
130+
if (option.startsWith('done date')) {
131+
taskLayoutOptions.setVisibility(TaskLayoutComponent.DoneDate, visible);
132+
return true;
133+
}
134+
if (option.startsWith('recurrence rule')) {
135+
taskLayoutOptions.setVisibility(TaskLayoutComponent.RecurrenceRule, visible);
136+
return true;
137+
}
138+
if (option.startsWith('tags')) {
139+
taskLayoutOptions.setTagsVisibility(visible);
140+
return true;
141+
}
142+
if (option.startsWith('id')) {
143+
taskLayoutOptions.setVisibility(TaskLayoutComponent.Id, visible);
144+
return true;
145+
}
146+
if (option.startsWith('depends on')) {
147+
taskLayoutOptions.setVisibility(TaskLayoutComponent.DependsOn, visible);
148+
return true;
149+
}
150+
if (option.startsWith('on completion')) {
151+
taskLayoutOptions.setVisibility(TaskLayoutComponent.OnCompletion, visible);
152+
return true;
153+
}
154+
return false;
155+
}

‎src/Query/Query.ts‎

Lines changed: 21 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { getSettings } from '../Config/Settings';
22
import type { IQuery } from '../IQuery';
3-
import { QueryLayoutOptions } from '../Layout/QueryLayoutOptions';
4-
import { TaskLayoutComponent, TaskLayoutOptions } from '../Layout/TaskLayoutOptions';
3+
import { QueryLayoutOptions, parseQueryShowHideOptions } from '../Layout/QueryLayoutOptions';
4+
import { TaskLayoutOptions, parseTaskShowHideOptions } from '../Layout/TaskLayoutOptions';
55
import { errorMessageForException } from '../lib/ExceptionTools';
66
import { logging } from '../lib/logging';
77
import { expandPlaceholders } from '../Scripting/ExpandPlaceholders';
@@ -27,24 +27,23 @@ export class Query implements IQuery {
2727

2828
private _limit: number | undefined = undefined;
2929
private _taskGroupLimit: number | undefined = undefined;
30-
private _taskLayoutOptions: TaskLayoutOptions = new TaskLayoutOptions();
31-
private _queryLayoutOptions: QueryLayoutOptions = new QueryLayoutOptions();
32-
private _filters: Filter[] = [];
30+
private readonly _taskLayoutOptions: TaskLayoutOptions = new TaskLayoutOptions();
31+
private readonly _queryLayoutOptions: QueryLayoutOptions = new QueryLayoutOptions();
32+
private readonly _filters: Filter[] = [];
3333
private _error: string | undefined = undefined;
34-
private _sorting: Sorter[] = [];
35-
private _grouping: Grouper[] = [];
34+
private readonly _sorting: Sorter[] = [];
35+
private readonly _grouping: Grouper[] = [];
3636
private _ignoreGlobalQuery: boolean = false;
3737

38-
private readonly hideOptionsRegexp =
39-
/^(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;
38+
private readonly hideOptionsRegexp = /^(hide|show) +(.*)/i;
4039
private readonly shortModeRegexp = /^short/i;
4140
private readonly fullModeRegexp = /^full/i;
4241
private readonly explainQueryRegexp = /^explain/i;
4342
private readonly ignoreGlobalQueryRegexp = /^ignore global query/i;
4443

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

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

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

305304
private parseHideOptions(line: string): void {
306305
const hideOptionsMatch = line.match(this.hideOptionsRegexp);
307-
if (hideOptionsMatch !== null) {
308-
const hide = hideOptionsMatch[1].toLowerCase() === 'hide';
309-
const option = hideOptionsMatch[2].toLowerCase();
310-
311-
switch (option) {
312-
case 'tree':
313-
// WARNING: undocumented, and not yet ready for release.
314-
this._queryLayoutOptions.hideTree = hide;
315-
break;
316-
case 'task count':
317-
this._queryLayoutOptions.hideTaskCount = hide;
318-
break;
319-
case 'backlink':
320-
this._queryLayoutOptions.hideBacklinks = hide;
321-
break;
322-
case 'postpone button':
323-
this._queryLayoutOptions.hidePostponeButton = hide;
324-
break;
325-
case 'priority':
326-
this._taskLayoutOptions.setVisibility(TaskLayoutComponent.Priority, !hide);
327-
break;
328-
case 'cancelled date':
329-
this._taskLayoutOptions.setVisibility(TaskLayoutComponent.CancelledDate, !hide);
330-
break;
331-
case 'created date':
332-
this._taskLayoutOptions.setVisibility(TaskLayoutComponent.CreatedDate, !hide);
333-
break;
334-
case 'start date':
335-
this._taskLayoutOptions.setVisibility(TaskLayoutComponent.StartDate, !hide);
336-
break;
337-
case 'scheduled date':
338-
this._taskLayoutOptions.setVisibility(TaskLayoutComponent.ScheduledDate, !hide);
339-
break;
340-
case 'due date':
341-
this._taskLayoutOptions.setVisibility(TaskLayoutComponent.DueDate, !hide);
342-
break;
343-
case 'done date':
344-
this._taskLayoutOptions.setVisibility(TaskLayoutComponent.DoneDate, !hide);
345-
break;
346-
case 'recurrence rule':
347-
this._taskLayoutOptions.setVisibility(TaskLayoutComponent.RecurrenceRule, !hide);
348-
break;
349-
case 'edit button':
350-
this._queryLayoutOptions.hideEditButton = hide;
351-
break;
352-
case 'urgency':
353-
this._queryLayoutOptions.hideUrgency = hide;
354-
break;
355-
case 'tags':
356-
this._taskLayoutOptions.setTagsVisibility(!hide);
357-
break;
358-
case 'id':
359-
this._taskLayoutOptions.setVisibility(TaskLayoutComponent.Id, !hide);
360-
break;
361-
case 'depends on':
362-
this._taskLayoutOptions.setVisibility(TaskLayoutComponent.DependsOn, !hide);
363-
break;
364-
case 'on completion':
365-
this._taskLayoutOptions.setVisibility(TaskLayoutComponent.OnCompletion, !hide);
366-
break;
367-
default:
368-
this.setError('do not understand hide/show option', new Statement(line, line));
369-
}
306+
if (hideOptionsMatch === null) {
307+
return;
308+
}
309+
const hide = hideOptionsMatch[1].toLowerCase() === 'hide';
310+
const option = hideOptionsMatch[2].toLowerCase();
311+
312+
if (parseQueryShowHideOptions(this._queryLayoutOptions, option, hide)) {
313+
return;
314+
}
315+
if (parseTaskShowHideOptions(this._taskLayoutOptions, option, !hide)) {
316+
return;
370317
}
318+
this.setError('do not understand hide/show option', new Statement(line, line));
371319
}
372320

373321
private parseFilter(line: string, statement: Statement) {

‎tests/Query/Query.test.ts‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,14 +84,22 @@ function isValidQueryGroup(filter: string) {
8484
lowerCaseFilterGaveExpectionInstruction(filter, query.grouping[0].instruction);
8585
}
8686

87-
function isInvalidQueryInstruction(getQueryError: (source: string) => string | undefined, source: string) {
88-
expect(getQueryError(source)).toEqual(`do not understand query
87+
function isInvalidQueryInstruction(
88+
getQueryError: (source: string) => string | undefined,
89+
source: string,
90+
expectedErrorMessage: string,
91+
) {
92+
expect(getQueryError(source)).toEqual(`${expectedErrorMessage}
8993
Problem line: "${source}"`);
9094
}
9195

92-
function isInvalidQueryInstructionLowerAndUpper(getQueryError: (source: string) => string | undefined, source: string) {
93-
isInvalidQueryInstruction(getQueryError, source);
94-
isInvalidQueryInstruction(getQueryError, source.toUpperCase());
96+
function isInvalidQueryInstructionLowerAndUpper(
97+
getQueryError: (source: string) => string | undefined,
98+
source: string,
99+
expectedErrorMessage: string = 'do not understand query',
100+
) {
101+
isInvalidQueryInstruction(getQueryError, source, expectedErrorMessage);
102+
isInvalidQueryInstruction(getQueryError, source.toUpperCase(), expectedErrorMessage);
95103
}
96104

97105
describe('Query parsing', () => {
@@ -452,6 +460,7 @@ describe('Query parsing', () => {
452460
'full',
453461
'full mode',
454462
'hide backlink',
463+
'hide backlinks',
455464
'hide cancelled date',
456465
'hide created date',
457466
'hide depends on',
@@ -476,6 +485,7 @@ describe('Query parsing', () => {
476485
'short',
477486
'short mode',
478487
'show backlink',
488+
'show backlinks',
479489
'show cancelled date',
480490
'show created date',
481491
'show depends on',
@@ -527,6 +537,18 @@ describe('Query parsing', () => {
527537
});
528538
});
529539

540+
it('should allow spaces between show or hide and a Query option', () => {
541+
const query1 = new Query('show tree');
542+
expect(query1.queryLayoutOptions.hideTree).toBe(false);
543+
expect(query1.error).toBeUndefined();
544+
});
545+
546+
it('should allow spaces between show or hide and a Task option', () => {
547+
const query1 = new Query('hide priority');
548+
expect(query1.taskLayoutOptions.isShown(TaskLayoutComponent.Priority)).toBe(false);
549+
expect(query1.error).toBeUndefined();
550+
});
551+
530552
it('should parse ambiguous sort by queries correctly', () => {
531553
expect(new Query('sort by status').sorting[0].property).toEqual('status');
532554
expect(new Query('SORT BY STATUS').sorting[0].property).toEqual('status');
@@ -615,7 +637,7 @@ Problem line: "${source}"`,
615637

616638
it('for invalid hide', () => {
617639
const source = 'hide nonsense';
618-
isInvalidQueryInstructionLowerAndUpper(getQueryError, source);
640+
isInvalidQueryInstructionLowerAndUpper(getQueryError, source, 'do not understand hide/show option');
619641
});
620642

621643
it('for unknown instruction', () => {

0 commit comments

Comments
 (0)