Add support for globs in excluded paths #2316
Conversation
Generated by 🚫 Danger |
|
Looking at how to fix the test length |
|
I moved the alias related configuration tests to a new file here to help with the length issue |
|
Happy to move something else, or do that in a separate PR if needed |
Codecov Report
@@ Coverage Diff @@
## master #2316 +/- ##
==========================================
+ Coverage 91.99% 92.01% +0.02%
==========================================
Files 296 299 +3
Lines 14886 14936 +50
==========================================
+ Hits 13694 13744 +50
Misses 1192 1192
Continue to review full report at Codecov.
|
| let includedPaths = included.flatMap { | ||
| fileManager.filesToLint(inPath: $0, rootDirectory: rootPath) | ||
| } | ||
| return (pathsForPath + includedPaths).filter { | ||
| !excludedPaths.contains($0) | ||
| } | ||
| } | ||
|
|
||
| private func resolveGlobs(in pattern: String) -> [String] { |
There was a problem hiding this comment.
Do you think it'd be worth it to extract this to a new class and add some tests directly?
There was a problem hiding this comment.
I left the first test since it's about Configuration calling glob, and added some new glob ones
| #endif | ||
|
|
||
| return (0..<Int(matchCount)).compactMap { index in | ||
| return String(validatingUTF8: globResult.gl_pathv[index]!) |
There was a problem hiding this comment.
what do you think about return globResult.gl_pathv[index].flatMap(String.init(validatingUTF8:)) just to avoid any potencial issues?
There was a problem hiding this comment.
Done, but FWIW in this case IMO it's better to crash since this would be breaking the API contract.
There was a problem hiding this comment.
Had to use it in a closure since it couldn't infer the type otherwise
keith
force-pushed
the
ks/exclude-glob
branch
3 times, most recently
from
92bdae3
to
144e4bf
Compare
|
This looks good to me from an implementation point of view. We probably should also update the README mentioning this and how to use it. |
|
Added |
|
Any other thoughts here @marcelofabri ? |
|
Thanks @keith! |
This does not support recursive globs, but it seems like for many cases
plain globs would be enough. This also means we don't need any libraries
to support this case.