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 matchpattern file: match #2782

Merged
merged 3 commits into from
Jan 9, 2018

Conversation

Sxderp
Copy link
Contributor

@Sxderp Sxderp commented Dec 28, 2017

I don't recall how I stumbled upon this, but I found out that MatchPattern.js doesn't quite follow Google's specification. file:///foo* is a valid match pattern but the previous code would not allow it. A fix to the Regex solved that problem.

Also added a bunch of tests.

@Eselce
Copy link
Contributor

Eselce commented Dec 28, 2017

TYPO: protcol

@Sxderp
Copy link
Contributor Author

Sxderp commented Dec 28, 2017

TYPO: protcol

Never happened. =-)

@arantius arantius added this to the 4.2 milestone Dec 29, 2017
= (pattern, urlStr) => assert.isNotOk(pattern.doMatch(new URL(urlStr)));
describe('invalid patterns', () => {
const newPattern =
pattern => { return () => new MatchPattern(pattern) };
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this fits on one line, but for all statements continued across >1 line, continuations should be indented 4 spaces.

Sxderp added a commit to Sxderp/greasemonkey that referenced this pull request Jan 6, 2018
@Sxderp Sxderp force-pushed the fix-matchpattern-file-match branch from 8a5e855 to 1370ac0 Compare January 6, 2018 16:16
Sxderp added a commit to Sxderp/greasemonkey that referenced this pull request Jan 6, 2018
@Sxderp Sxderp force-pushed the fix-matchpattern-file-match branch from 1370ac0 to aa8d6d1 Compare January 6, 2018 16:23
@arantius arantius modified the milestones: 4.2, 4.x Jan 6, 2018
@arantius
Copy link
Collaborator

arantius commented Jan 6, 2018

This is valuable, but only after #2693 .

@Sxderp
Copy link
Contributor Author

Sxderp commented Jan 6, 2018

This is valuable, but only after #2693 .

It's the other way around. This PR, fixing MatchPattern #2782, needs to be merged before #2693 (or any PRs that fix it).

There's no point in fixing / allow #2693 if MatchPattern doesn't properly match against them.

@arantius
Copy link
Collaborator

arantius commented Jan 8, 2018

if only #2693 is done you can still use @include, and get execution of scripts at local files.

If you only do this, you get nothing. But yes, the two are closely related.

@Sxderp
Copy link
Contributor Author

Sxderp commented Jan 8, 2018

if only #2693 is done you can still use @include

Heh, I forgot about @include. I'll rebase the commits.

@arantius arantius modified the milestones: 4.x, 4.2 Jan 9, 2018
@arantius arantius merged commit aa8d6d1 into greasemonkey:master Jan 9, 2018
@Sxderp Sxderp deleted the fix-matchpattern-file-match branch January 11, 2018 00:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants