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

feat: change ignore rules to match whole name #922

Closed
wants to merge 1 commit into from

Conversation

mattlyons0
Copy link
Contributor

Previously ignore rules would be fuzzy compared (a rule of '.git' would match 'myproject.git') and have unintended consequences.
Fixes #916

@mattlyons0
Copy link
Contributor Author

mattlyons0 commented Oct 15, 2016

This may break some configurations where users were taking advantage of the fact that the ignore rules matched fuzzy. This affects both the default configuration of ignore rules as well as user specified.

See my comment on #916 for why I think the old behavior should be changed. The short version is that if the full path of a project folder had a folder with '.git', 'coverage' or any of the folders in the ignore-by-default package as a substring of the folder name the entire directory tree up from that folder would be ignored. Folders like 'mycoverageapplication' would be ignored entirely with the old behavior and default settings. The same principle applies to files. There could still be an issue if somewhere in the file tree a user puts their code in a folder named explicitly 'coverage' or 'node_modules' however I don't think that happens very often and would be more obvious than previous behaviors matching.

I believe that the regex should support windows style '' file paths however I am unable to verify this.

@mattlyons0 mattlyons0 force-pushed the working branch 2 times, most recently from 03547c0 to 335ee07 Compare October 15, 2016 00:40
Previously ignore rules would be fuzzy compared (a rule of '.git' would match 'myproject.git') and have unintended consequences.
Fixes remy#916
@rjmunro
Copy link
Contributor

rjmunro commented Oct 15, 2016

Does this apply to all ignore rules or only the default (ignoreRoot) ones?

@mattlyons0
Copy link
Contributor Author

All ignore rules, user configured and ignoreRoot.

@remy
Copy link
Owner

remy commented Oct 16, 2016

Is it possible to add some tests directly to this function, at least to show it does what it says on the tin?

@remy
Copy link
Owner

remy commented Sep 4, 2017

I know this PR is old, and I'm going to close it for now, since I suspect you've possibly moved on. But I'd be very happy to re-open and merge if a test can be added to confirm the expected functionality.

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