Make negated pattern position-insensitive by valango · Pull Request #30 · sindresorhus/matcher

valango · 2021-03-08T17:49:38Z

Maybe I was a bit hasty w this one, but it was blocking my long-dormant #25.

Fixes #29

sindresorhus · 2021-03-10T09:25:02Z

You also need to update .isMatch.

sindresorhus · 2021-03-10T09:22:31Z

 		for (const pattern of patterns) {
-			if (pattern.test(input)) {
-				matches = !pattern.negated;
+			if (pattern.test(input) && !(matches = !pattern.negated)) {


I don't like having an assignment in a condition. Move it out into a separate if.

sindresorhus · 2021-03-10T09:22:49Z

-Use `*` to match zero or more characters. A pattern starting with `!` will be negated.
+Use `*` to match zero or more characters.
+
+A leading `!` negates the pattern - any matching inputs will be excluded from the results.


You need to update index.d.ts too.

sindresorhus · 2021-03-10T09:23:03Z

 	t.deepEqual(matcher(['foo', 'bar'], ['foo']), ['foo']);
 	t.deepEqual(matcher(['foo', 'bar'], ['bar']), ['bar']);
 	t.deepEqual(matcher(['foo', 'bar'], ['fo*', 'ba*', '!bar']), ['foo']);
+	t.deepEqual(matcher(['foo', 'bar'], ['fo*', '!bar', 'ba*']), ['foo']);


This change will need a lot more assertions.

valango · 2021-03-10T19:21:25Z

Ok, Im almost there... just noticed that in the current index.d.ts, the @returns declarations are switched - I'll correct that, too.

For the matcher(), I'd add a horribly long wording - provided you want both .ts and .md exactly equivalent:

@param patterns - String or array of string patterns. A leading ! negates the pattern. An input string will be omitted, if it does not match any non-negated patterns present, or if it matches a negated pattern, or if no pattern is present.

Please confirm or suggest a better wording - then I'll submit my final version.

sindresorhus · 2021-03-22T09:14:20Z

String or array of string patterns.

This part feels moot in the TS definition (but not the readme) as it's already made clear by the types.

Otherwise, it looks ok.

valango · 2021-03-28T13:57:37Z

Plz check & let me know. I did not bump the version.

sindresorhus · 2021-04-19T10:11:54Z

 		}

-		if (matches) {
+		if (matches || (matches === undefined && !patterns.some(p => !p.negated))) {


Suggested change

if (matches || (matches === undefined && !patterns.some(p => !p.negated))) {

if (matches || (matches === undefined && !patterns.some(pattern => !pattern.negated))) {

sindresorhus · 2021-04-19T10:12:24Z

+A leading `!` negates the pattern.
+
+An input string will be omitted, if it does not match any non-negated patterns present,
+or if it matches a negated pattern, or if no pattern is present.


Don't hard-wrap.

sindresorhus · 2021-04-19T10:12:49Z

-Use `*` to match zero or more characters. A pattern starting with `!` will be negated.
+Use `*` to match zero or more characters.
+
+A leading `!` negates the pattern.


Readme and index.d.ts should be in sync documentation-wise.

"This part feels moot in the TS definition (but not the readme) as it's already made clear by the types."

Did I get you wrong or have you changed your mind?

Ok, let me rephrase: index.d.ts and readme should be in sync, except for cases where the readme just points out the types as that's already clear by the types.

However, that's not what I'm pointing out here. You have removed some stuff from index.d.ts that is preserved here and not updated other parts, like https://github.com/sindresorhus/matcher/pull/30/files#diff-7aa4473ede4abd9ec099e87fec67fd57afafaf39e05d493ab4533acc38547eb8R83

Ok, I restored the missing stuff and made the contents as coherent as I could via ee30306.
I also found:

'inputs' and 'patterns' were randomly used in both plural and singular form. Fixed that.

'@param options' is missing in index.d.ts - I left it for you to decide.

Ok, I restored the missing stuff and made the contents as coherent as I could via ee30306.
Also found:

'inputs' and 'patterns' were randomly used in both plural and singular form. Fixed that.

'@param options' is missing in index.d.ts - I left it for you to decide.

sindresorhus · 2021-04-19T10:13:40Z

@@ -37,6 +41,10 @@ test('matcher.isMatch()', t => {
 	t.true(matcher.isMatch('moo', ['*oo', '!f*']));
 	t.true(matcher.isMatch('UNICORN', ['!*oo', 'UNI*'], {caseSensitive: true}));



Move these to a separate test with a descriptive title of what they're testing. Same with the other assertions.

Did so. New tests named "... negated patern placement".

sindresorhus · 2021-05-02T13:07:11Z

-			return regexp.negated ? !matches : matches;
-		});
-	});
+	return matching.length > 0;


The problem with this new logic here is that it's inefficient. The old way, it short-circuited when it found a match. Now it will do everything and then check for a match.

Good point. I modified the code. Now isMatch seems to be bit quicker compared to the original implementation, probably because not constructing Regxp repeatedly.

sindresorhus · 2021-05-05T15:33:13Z

-		return false;
-	}
+module.exports.isMatch = (inputs, patterns, options) => {
+	const matching = module.exports(inputs, patterns, options, true);


This doesn't change anything as you're calling the incorrect method.

Omg, made error on merge. Fixed now.

sindresorhus · 2021-05-07T13:05:32Z

You still need to do #30 (comment).

Negated rule made position-insensitive (sindresorhus#29)

6fbb9a6

sindresorhus requested changes Mar 10, 2021

View reviewed changes

edited: formatting, documentation and tests

812edd9

sindresorhus requested changes Apr 19, 2021

View reviewed changes

valango and others added 5 commits April 30, 2021 13:46

fixed: double isMatch definition

680c03f

fixed: code wrapping

d6d2224

fixed: readme hard-wrap

1dcbd5a

new tests separated

45ce7a3

Update index.js

7b94813

sindresorhus reviewed May 2, 2021

View reviewed changes

improved: isMatch performance

ea345a5

sindresorhus reviewed May 5, 2021

View reviewed changes

fixed: merge error

ca77fbb

valango and others added 2 commits May 9, 2021 14:34

fixed: controversies btw index.d.ts and readme.md

ee30306

Merge branch 'main' into ticket_29

900e0b3

sindresorhus changed the title ~~Negated rule made position-insensitive (#29)~~ Make negated pattern position-insensitive May 12, 2021

sindresorhus merged commit 43d778c into sindresorhus:main May 12, 2021

	if (matches \|\| (matches === undefined && !patterns.some(p => !p.negated))) {
	if (matches \|\| (matches === undefined && !patterns.some(pattern => !pattern.negated))) {

		@@ -37,6 +41,10 @@ test('matcher.isMatch()', t => {
		t.true(matcher.isMatch('moo', ['oo', '!f']));
		t.true(matcher.isMatch('UNICORN', ['!oo', 'UNI'], {caseSensitive: true}));

Uh oh!

Conversation

valango commented Mar 8, 2021 • edited by sindresorhus Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sindresorhus commented Mar 10, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

valango commented Mar 10, 2021

Uh oh!

sindresorhus commented Mar 22, 2021

Uh oh!

valango commented Mar 28, 2021

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

valango May 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sindresorhus commented May 7, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

valango commented Mar 8, 2021 •

edited by sindresorhus

Loading

valango May 9, 2021 •

edited

Loading