-
Notifications
You must be signed in to change notification settings - Fork 40
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
Ensure compliance with git. #66
base: master
Are you sure you want to change the base?
Conversation
Thank you. Unfortunately, some tests are failing. Also, there is a lot of superfluous output.
|
Ohh « It is not possible to re-include a file if a parent directory of that file is excluded. ». Also we may have hard times spotting directories, as if (pattern->flags & PATTERN_FLAG_MUSTBEDIR) {
*dtype = resolve_dtype(*dtype, istate, pathname, pathlen);
if (*dtype != DT_DIR)
continue;
} |
I know the tests are failing, but it has to be fixed, they are failing because gitignore_parser behave differently than gitignore. I played a bit with the code and went down to a single error instead of 4, but the error is with the exclusion rules, and I can't wrap my head around it yet... I pushed my failures here: https://github.com/JulienPalard/gitignore_parser/tree/mdk-playing To be honest I don't really know if my modifications enhance anything, it enhance the current tests, but adding more tests could reveal more subtleties, I don't know. |
@mherrmann if you are interested in full (or at least more) git compliance, I've been working on a collection of corner cases tests for a similar project. Here is a gist with slight adjustments to run standalone.
IMPORTANT: apply |
Thank you @astos-marcb. I don't have time to proactively investigate edge cases that gitignore_parser might handle incorrectly. I will be happy to accept PRs that add test cases for and / or fix them. |
This add a
TestCase
to run all tests against git, thus ensuring that current tests, and all future tests, are compliant with git.