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(files): Ignore included:false pattern #1974

Merged
merged 1 commit into from
Mar 18, 2016

Conversation

budde377
Copy link
Member

@budde377 budde377 commented Mar 7, 2016

Do not include a file, when include=false.
Give include=false precedence over include=true.

Closes #1530

files = [
{pattern: 'files/log_foo.js', included: false},
{pattern: 'files/log_foo.js', included: true},
'files/*.js'
Copy link
Member

Choose a reason for hiding this comment

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

It would be good to have one test case where files/*.js is the first entry.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

@budde377
Copy link
Member Author

budde377 commented Mar 7, 2016

@dignifiedquire There's an integration test that fails, because it assumes some other semantic than the proposed precedence. Should the test be changed or should the semantics?

@dignifiedquire
Copy link
Member

@budde377 right, so we have to think about this carefully, because in the 0.12 and before the semantics where that the most specific pattern would match, rather than false > true. So ideally I thought we could revert to that, what do you think?

@budde377
Copy link
Member Author

budde377 commented Mar 7, 2016

@dignifiedquire. Lets do this the cool way! Inspired by the CSS specificity calculations, we should define a specificity-measure. I propose the following:

Minimatch supports:

  • Globstar
  • Wildcard
  • Extended glob matching
  • Brace expansion

Given a path, p, we can define a tuple s(p) = (ng, nw, ne, nb) where ng, nw, ne, and nb are the number of globstars, wildcards, extended glob match patterns, and brace expansion patterns, respectively.

With any two paths, p and q, where s(p)=(p1, p2, p3, p4) and s(q)=(q1, q2, q3, q4) we can define a order of the corresponding tuples

p1 < p2 ⇔ ∃ m > 0: ∀ i > m: pi = qi ∧ pm < qm

i.e. an lexicographical order.

Given two paths with different tuples, we can define the one with the smallest tuple (wrt. the order above) as the most specific. If the tuples are equal we can choose either one.

@budde377
Copy link
Member Author

budde377 commented Mar 8, 2016

Or we could just let the number of files generated from a given pattern decide how specific it is. The more files, the less specific.

@dignifiedquire
Copy link
Member

@budde377 💯 for the cool order you suggested above. We just need to make sure to document and test this well, so people understand what's happening :)

@budde377
Copy link
Member Author

budde377 commented Mar 8, 2016

@dignifiedquire Yes. I'll start on it this afternoon.

@dignifiedquire dignifiedquire added this to the v1.0 milestone Mar 9, 2016
@budde377
Copy link
Member Author

After some failed attempts, I'm pretty sure that I have a solution. I'll have it implemented and tested primo next week.

@dignifiedquire
Copy link
Member

Thanks @budde377 for the great work, looking forward to see it :)

Define specificity of patterns.
Let most specific pattern decide to include files or not.

Closes karma-runner#1530
@budde377
Copy link
Member Author

@dignifiedquire I've now implemented an order. Please let me know if it needs more documentation etc.

@dignifiedquire
Copy link
Member

Nice work, will review more closely tomorrow

@dignifiedquire dignifiedquire self-assigned this Mar 17, 2016
it('should calculate right weight of pattern with more magic', () => {
helper.mmPatternWeight('{0..9}/?(a|b)c/[abc]/**/*.jsx?').should.be.deep.equal([10, 1, 1, 1, 1, 1])
})
it('should calculate right weight of pattern as worst glob set', () => {
Copy link
Member

Choose a reason for hiding this comment

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

worst glob set

👍 😆

@dignifiedquire
Copy link
Member

Thanks @budde377 looking good, going to merge this and hope for the best :)

dignifiedquire added a commit that referenced this pull request Mar 18, 2016
fix(files): Ignore included:false pattern
@dignifiedquire dignifiedquire merged commit b2cb31a into karma-runner:master Mar 18, 2016
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.

2 participants