-
Notifications
You must be signed in to change notification settings - Fork 6
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
Use re.match(pattern, include)
for ignored include patterns
#77
Conversation
@martis42 Maybe we should document the code that was used to match against ignored include patterns and provides some more examples. |
ce4caae
to
68e5a74
Compare
I believe you have a point. Still, I would like to think this through in detail. For today my time budget for DWYU is exhausted. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@storypku
I agree with your proposal. This is indeed the preferable behavior. I will take care of improving the documentation after this has been merged.
However, please change the test case as requested.
68e5a74
to
90622dc
Compare
Done. Please help re-review. |
@@ -2,6 +2,6 @@ | |||
"ignore_include_patterns": [ | |||
"foo/.*", | |||
"bar/.*", | |||
"./ignored_sub_path/.*" | |||
"\\w+/ignored_sub_path/.*" | |||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add also an acceptance test case here for ingore_include_patterns to mismatch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good proposal. I will do so.
@@ -2,6 +2,6 @@ | |||
"ignore_include_patterns": [ | |||
"foo/.*", | |||
"bar/.*", | |||
"./ignored_sub_path/.*" | |||
"\\w+/ignored_sub_path/.*" | |||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good proposal. I will do so.
If instead we use `re.search(pattern, include)`, include `baz/a/b.h` will be matched against pattern 'a/.*', which should be an unexpected behavior.
90622dc
to
774f483
Compare
If instead we use
re.search(pattern, include)
, includebaz/a/b.h
will be matched against patterna/.*
, which should be an unexpected behavior.