Skip to content

Commit 6ae98cf

Browse files
authored
refactor: Extract text searching out of TextField (#1054)
* refactor: Add SubstringMatcher class This will be used to simplify TextField's implementation * refactor: Adjust TextField to use SubstringMatcher * refactor: Add RegexMatcher class This will be used to simplify TextField's implementation * refactor: TextField now uses RegexMatcher * refactor: RegexMatcher now validates user input * refactor: Better parameter name in SubstringMatcher * refactor: Extract interface StringMatcher * refactor: RegexMatcher implements StringMatcher * refactor: Rename StringMatcher to IStringMatcher * refactor: Fix typo in method name validateAndStruct() * comment: Add JSDoc docs for new classes * refactor: Add public keyword to make method visibility explicit * refactor: Remove cyclic dependency between TextField and SubstringMatcher by moving the implementation from TextField to SubstringMatcher, but leaving the original in place as a thin wrapper around the new one, to localise the change.
1 parent 3773686 commit 6ae98cf

File tree

6 files changed

+142
-20
lines changed

6 files changed

+142
-20
lines changed

src/Query/Filter/TextField.ts

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import type { Task } from '../../Task';
2+
import { SubstringMatcher } from '../Matchers/SubstringMatcher';
3+
import { RegexMatcher } from '../Matchers/RegexMatcher';
24
import { Field } from './Field';
35
import { FilterOrErrorMessage } from './Filter';
46

@@ -14,30 +16,24 @@ export abstract class TextField extends Field {
1416
if (match !== null) {
1517
const filterMethod = match[1];
1618
if (['includes', 'does not include'].includes(filterMethod)) {
17-
result.filter = (task: Task) =>
18-
TextField.maybeNegate(
19-
TextField.stringIncludesCaseInsensitive(
20-
this.value(task),
21-
match[2],
22-
),
19+
const matcher = new SubstringMatcher(match[2]);
20+
result.filter = (task: Task) => {
21+
return TextField.maybeNegate(
22+
matcher.matches(this.value(task)),
2323
filterMethod,
2424
);
25+
};
2526
} else if (
2627
['regex matches', 'regex does not match'].includes(filterMethod)
2728
) {
28-
// Courtesy of https://stackoverflow.com/questions/17843691/javascript-regex-to-match-a-regex
29-
const regexPattern =
30-
/\/((?![*+?])(?:[^\r\n[/\\]|\\.|\[(?:[^\r\n\]\\]|\\.)*])+)\/((?:g(?:im?|mi?)?|i(?:gm?|mg?)?|m(?:gi?|ig?)?)?)/;
31-
const query = match[2].match(regexPattern);
32-
33-
if (query !== null) {
34-
result.filter = (task: Task) =>
35-
TextField.maybeNegate(
36-
this.value(task).match(
37-
new RegExp(query[1], query[2]),
38-
) !== null,
29+
const matcher = RegexMatcher.validateAndConstruct(match[2]);
30+
if (matcher !== null) {
31+
result.filter = (task: Task) => {
32+
return TextField.maybeNegate(
33+
matcher.matches(this.value(task)),
3934
filterMethod,
4035
);
36+
};
4137
} else {
4238
result.error = `cannot parse regex (${this.fieldName()}); check your leading and trailing slashes for your query`;
4339
}
@@ -54,9 +50,7 @@ export abstract class TextField extends Field {
5450
haystack: string,
5551
needle: string,
5652
): boolean {
57-
return haystack
58-
.toLocaleLowerCase()
59-
.includes(needle.toLocaleLowerCase());
53+
return SubstringMatcher.stringIncludesCaseInsensitive(haystack, needle);
6054
}
6155

6256
protected filterRegexp(): RegExp {

src/Query/Matchers/IStringMatcher.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
/**
2+
* An interface for determining whether a string value matches a particular condition.
3+
*
4+
* This is used to hide away the details of various text searches, such as the
5+
* simple inclusion of a sub-string, or the more complex regular expression searches.
6+
*/
7+
export interface IStringMatcher {
8+
/**
9+
* Return whether the given string matches this condition.
10+
* @param stringToSearch
11+
*/
12+
matches(stringToSearch: string): boolean;
13+
}

src/Query/Matchers/RegexMatcher.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import type { IStringMatcher } from './IStringMatcher';
2+
3+
/**
4+
* Regular-expression-based implementation of IStringMatcher.
5+
*/
6+
export class RegexMatcher implements IStringMatcher {
7+
private readonly regex: RegExp;
8+
9+
/**
10+
* Construct a RegexMatcher object.
11+
*
12+
* @param regex {RegExp} - A valid regular expression
13+
*/
14+
public constructor(regex: RegExp) {
15+
this.regex = regex;
16+
}
17+
18+
/**
19+
* Construct a RegexMatcher object if the supplied string is a valid regular expression.
20+
* and null if not.
21+
*
22+
* @param {string} regexInput - A string that can be converted to a regular expression.
23+
* It must begin with a /, and end either with / and optionally any
24+
* valid flags.
25+
*/
26+
public static validateAndConstruct(
27+
regexInput: string,
28+
): RegexMatcher | null {
29+
// Courtesy of https://stackoverflow.com/questions/17843691/javascript-regex-to-match-a-regex
30+
const regexPattern =
31+
/\/((?![*+?])(?:[^\r\n[/\\]|\\.|\[(?:[^\r\n\]\\]|\\.)*])+)\/((?:g(?:im?|mi?)?|i(?:gm?|mg?)?|m(?:gi?|ig?)?)?)/;
32+
const query = regexInput.match(regexPattern);
33+
34+
if (query !== null) {
35+
const regExp = new RegExp(query[1], query[2]);
36+
return new RegexMatcher(regExp);
37+
} else {
38+
return null;
39+
}
40+
}
41+
42+
public matches(stringToSearch: string): boolean {
43+
return stringToSearch.match(this.regex) !== null;
44+
}
45+
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import type { IStringMatcher } from './IStringMatcher';
2+
3+
/**
4+
* Substring-based implementation of IStringMatcher.
5+
*
6+
* This does a case-insensitive search for the given string.
7+
*/
8+
export class SubstringMatcher implements IStringMatcher {
9+
private readonly stringToFind: string;
10+
11+
/**
12+
* Construct a SubstringMatcher object
13+
*
14+
* @param {string} stringToFind - The string to search for.
15+
* Searches will be case-insensitive.
16+
*/
17+
public constructor(stringToFind: string) {
18+
this.stringToFind = stringToFind;
19+
}
20+
21+
public matches(stringToSearch: string): boolean {
22+
return SubstringMatcher.stringIncludesCaseInsensitive(
23+
stringToSearch,
24+
this.stringToFind,
25+
);
26+
}
27+
28+
public static stringIncludesCaseInsensitive(
29+
haystack: string,
30+
needle: string,
31+
): boolean {
32+
return haystack
33+
.toLocaleLowerCase()
34+
.includes(needle.toLocaleLowerCase());
35+
}
36+
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { RegexMatcher } from '../../../src/Query/Matchers/RegexMatcher';
2+
3+
describe('RegexMatcher', () => {
4+
it('should match simple regular expression', () => {
5+
const regex: RegExp = /^hello world$/;
6+
const matcher = new RegexMatcher(regex);
7+
expect(matcher.matches('hello world')).toStrictEqual(true);
8+
expect(matcher.matches('xx hello world yy')).toStrictEqual(false);
9+
expect(matcher.matches('search me')).toStrictEqual(false);
10+
});
11+
12+
it('should construct regex from a valid string', () => {
13+
const matcher = RegexMatcher.validateAndConstruct('/hello world/i');
14+
expect(matcher).not.toBeNull();
15+
expect(matcher!.matches('hello world')).toStrictEqual(true);
16+
expect(matcher!.matches('HELLO world')).toStrictEqual(true); // Confirm that the flag 'i' is retained
17+
expect(matcher!.matches('Bye Bye')).toStrictEqual(false);
18+
});
19+
20+
it('should not construct regex from an INvalid string', () => {
21+
const matcher = RegexMatcher.validateAndConstruct('I have no slashes');
22+
expect(matcher).toBeNull();
23+
});
24+
});
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import { SubstringMatcher } from '../../../src/Query/Matchers/SubstringMatcher';
2+
3+
describe('SubstringMatcher', () => {
4+
it('should match simple text', () => {
5+
const matcher = new SubstringMatcher('find me');
6+
expect(matcher.matches('find me')).toStrictEqual(true);
7+
expect(matcher.matches('prefix find me suffix')).toStrictEqual(true);
8+
expect(matcher.matches('FIND ME')).toStrictEqual(true);
9+
});
10+
});

0 commit comments

Comments
 (0)